Provide an alternative for the api key check subscriber

This is rooted back in these two issues 🐛 Getting an error on chapgpt configure page. Fixed and 🐛 If OpenAI is unconfigured error in CKEditor plugin Fixed and a lot of Slack posts over the last year. People keep installing the module and not configuring it, and they also don't seem to be checking the reports page to see the requirement error. If there is a better way, I am all for it.

Is it primarily related to CK Editor? If so, I believe we could add validation that prevents saving if OpenAI is enabled in text format yet the configuration is not set. Ideally this merge request would also then cover sub-module configuration pages to prevent saving configuration when main module is not configured.

Would you be willing to work on that @kriboogh?

I guess the challenge of this issue would be to come up with a comprehensive enough list to avoid a future new deluge of support requests.

So the above merge request removes the event subscriber.

Instead, a wrapper is made around the OpenAI\Client, where each contract call is checked if the config is set or not. If not a new exception is thrown (ApiKeyNotConfiguredException). This exception is then catched by the OpenAiApi class, and logs the exception as before but also displays a messenger error if the user has permissions to configure the settings.
I also took the opportunity to use the interface (ClientContract) instead of the class for Client in the constructors. Same for the return types of the OpenAiAPi methods (they should return the Contract interfaces, not the classes).

I noticed some code instantiating the openai.client service directly instead of using the OpenAiApi service. Code doing that, should be responsible for dealing with exceptions in the contract calls itself. So this patch doesn't cover that (yet). Only if you call the OpenAiApi methods, the message will be shown.

There is also two helpers methods added, to prepareParameters and processResults for Contract calls being made.
This could in the future be used to send out events (for example OpenAiPrepareParametersEvent and OpenAiProcessResultsEvent) so other modules could hook into that. This is not yet implemented and perhaps is not in scope of this issue.

Thanks for doing that! I added some feedback on the merge request, but in general I am not confident whether this level of change replacing part of what is in the openai php client is something that would work for @kevinquillen as per my last comment on the MR - will have to leave to him to comment, so leaving as needs review.

Thanks

Indeed, I first added the catches to the \Drupal\openai\OpenAIApi class, but the problem (if you can call it that) is that I noticed some code is calling the client service directly, not using the wrapping OpenAiApi class. So any code using the service directly would bypass the checks. By doing it in the Client, all code is covered. But as you mentioned, it creates a dependency on the openai php client class.

Thanks for the details! Probably the solution there is to make sure we are calling the OpenAI service not the php client directly. The embeddings sub-module has an MR for that already. Was it more widespread than that? If so maybe we need a seperate issue and postpone this one until that is sorted.

Haven't experimented with other modules outside the "openai" ones. But I though to generalise it, since it's a public service you can use it directly, only then you will need to catch the exception yourself if you don't want a wsod. All code in this module should use the api class though. In that case we could then just deal with all of this in the api class and let other modules figure it out themselves if they decide to directly use the client. It's also a possible route to take of course.