Fix Exception

[yiranwu0]

Why are these changes needed?

Fix exceptions of non-OpenAI generations:

1. Remove meaningless exceptions that just catches the error and throw it. This also adds confusion to debugging when an error happens.
2. Remove retries: Some retries are also useless. If we catch any errors and throw it, the retry is not needed. If no error is caught, we don't need retry.

Fix Gemini retry:

1. Remove retry in GeminiClient. It should be in OpenAIWrapper.
2. OpenAIWrapper only catches OpenAI rate limits. This PR adds catches of google's rate limits so that different keys can be tried.

Future todo:

1. Add exception catches of non-openai models to OpenAIWrapper. Currently only gemini is added.
2. (Optional Feature): Add a wait time option for OpenAIWrapper to wait out the rate limits.

Related issue number

Checks

• I've included any doc changes needed for https://autogen-ai.github.io/autogen/. See https://autogen-ai.github.io/autogen/docs/Contribute#documentation to build and test documentation locally.
• I've added tests (if relevant) corresponding to the changes introduced in this PR.
• I've made sure all auto checks have passed.

[Hk669]

thanks @yiranwu0 for the changes. can you confirm if the updated code is working as expected with all the clients. have you tested it? if not @marklysze and i can give it a try.

cc @marklysze

[marklysze]

LGTM. @marklysze could you take a look too?

Yep, I'll have a look and test now.

[marklysze]

Thanks for raising this PR @yiranwu0, better bubbling of exceptions and centralised retries is definitely needed with all the client classes in place.

From what I understand from the code changes, the client-specific exceptions are added to client.py and then handled in the try/except around the client.create(params). Then it's logged and raised if it's the last client.

I was testing with Bedrock and checked out the exceptions that could be raised, and there's quite a lot of possible exceptions (boto and aws exceptions and click Click to see a full list of static exceptions). So, I was wondering whether, rather than trying to add each client's full list of exceptions into client.py and catch them, would it be possible to create either a generic ClientException class or an exception class in each client class (e.g. BedrockClientException, GeminiClientException) and then raise that, if we want to handle client-specific exceptions. Other options are to create our own set of exceptions for specific common scenarios, like ClientRateLimitException, and then we raise those specific ones from the client class and any others can be generic, like ClientException.

Amazon Bedrock is probably going to be the one with the most exceptions, though Anthropic with Bedrock may also have a few. Additionally, some exceptions may have the same name, so if we continue with the proposed approach then perhaps we should prefix them, e.g. InternalServerError becomes GeminiInternalServerError.

Let me know what you think...

[yiranwu0]

Hello @marklysze, thank you for testing it!
I don't think we need to attend to all exceptions from different clients, we only want to catch the few "time rate limits" exception and handle that in client.py because we can try different configs. For other exceptions, I think we should raise the original exception, so that user can debug from that, or search online.

However, I think it is good if we can have all the rate limit exceptions wrapped in an exception of our own, so that we only need to catch one exception. But that needs more thinking and could be done in the next PR.

[marklysze]

Thanks @yiranwu0, sounds like a plan! I'll look at the rest of the client classes and test :)

[marklysze]

Hey @yiranwu0, I've updated the code as follows:

• Removed try/except from Ollama client
• Added main exceptions for non-OpenAI clients into client.py
• Prefixed exceptions with the client name, e.g. gemini_InternalServerError and anthorpic_InternalServerError, to avoid conflicts
• Changed variables for exceptions in ImportError to be of type Exception instead of None as None can't be used in an except clause. Also moved the except clause to the end of the three exception blocks.
• Tested non-function and function workflows with the following (to ensure they are still working): Anthorpic, Cohere, Groq, Mistral, Ollama, Together.AI. (I haven't tested Gemini).
• Added a note for users to also install fix-busted-json if trying to use the Ollama client.

It caught exceptions when they arose. Though I can't test all exceptions.

[yiranwu0]

Hello @marklysze, your change looks good! I already tested Gemini. If there are good, we can merge it!

[marklysze]

Hello @marklysze, your change looks good! I already tested Gemini. If there are good, we can merge it!

Okay, great... I'm good with it!

[Hk669]

looks good to me. Thanks @yiranwu0 and @marklysze