Better error handling in the Chatbot

On AiQuotaException write a message that there are techincal issues and talk with an administrator.
On AiSetupFailureException write a message that there are techincal issues and talk with an administrator (this should not be able to happen really).

What are these? Is QuotaException running out of credits? If so

"I am sorry, something went wrong. This could be due to the attached account requiring more credits."

As mrdalesmith mentioned in the other comment:

"Just to note, the AI Key is checked as it is saved against the provider settings form for OpenAI so this particular error would be best handled by the providers (as each provider may need a different process to check if the key is usable).

Do all providers return these errors in a normalised way we can check for consistently?

"

The AI module defines a bunch of Exceptions that the provider modules can use to report back errors from the LLM itself in a consistent way. Exactly how and when they're used is up to each individual provider module but the idea is that they use them as consistently as possible so that the AI module can do something useful (in this case, give a relevant error message to the user).

AiQuotaException is not enough credits on the account.

AiSetupFailureException is meant for when the Drupal side (/connection to LLM) isn't working. The only place I can see it used at the moment is when the API key is saved in Drupal but for some reason it couldn't be loaded. So something is wrong with the setup on the Drupal side rather than with the connection, the LLM or the prompt.

@marcus_johansson As discussed, here's an MR. Some notes:

1. This introduces a minor backwards-compatibility issue by changing the type of Exception thrown. As discussed, we think this is probably okay as it would require other code to be doing custom error handling while calling the AiAssistantApiRunner.
2. I have not included an update hook to set any of the new custom messages. This is intentional because telling end users, for example, that an account is being rate limited or had reached a quota limit may be revealing more information than a site administrator would like. Therefore adding custom messages for this should be a manual change.
3. This MR doesn't change whether the Retry button appears. Firstly, the way deepchat handles errors does not allow for extra properties to be added to the error parameter in the error handler so any matching in the JS would have to be on the message itself (which could be done if we pass all error messages to drupalSettings but that feels messy and not very robust). Secondly, the first thing an end user is going to do when they see an error is try the exact same question again regardless of what the error message actually says so we should make it easy for them to do this.

Thoughts and responses welcome :)

Thanks Mark! I wrote some comments on it, only one is a must to change (use default error_message) - feel free to look through the others.

Two questions in general:

1. Does it make more sense to set the error messages in the actual block, rather then the assistants API, since its the ? This is a very open question and I don't have a clear opinion on it - the reason I ask is that you can in theory have the same assistant be connected to two block, one for an editor and one for an admin and they might be privy to different error messages.
2. The other question can be a follow-up ticket, since this solves the issue with Drupal CMS, where we control the configuration - we should add a form where a Drupal user can change this.

I've fixed the small things raised on the MR.

For both of the bigger questions I think we need to think about where the line between Assistant and Chatbot/Block should be in general (probably in a follow-up issue rather than hold this up).

I think I slightly lean the other way. The Assistant should be in charge of the error message (and therefore config) unless the throwException option is set to TRUE and then the code using it does have to handle the errors itself and may use the Assistant error message config if it wishes. In which case we should update AiAssistantApiRunner::process() to use the config when throwException is FALSE. But it's not a strongly-held belief and I could be convinced of the opposite so we should probably get some other opinions. I'm not sure it's worth holding the rest of this up for though?

Thank you Mark, it looks good now - its getting merged.

Automatically closed - issue fixed for 2 weeks with no activity.