fix(auto_client): stop masking runtime ImportErrors in from_provider

[yurekami]

Summary

Fixes #1940

When using a SOCKS proxy with the Google provider, from_provider("google/...") would incorrectly raise a ConfigurationError about missing google-genai package, even when the package was installed. The actual issue was a missing socksio dependency for httpx's SOCKS support.

Root cause: The except ImportError block caught ALL import errors, including runtime ones raised during genai.Client() initialization (e.g., httpx raising ImportError when socksio is missing for SOCKS proxy).

Fix: Separated import error handling from client initialization for all affected providers (google, vertexai, generative-ai):

• First try/except ImportError now only catches actual package import errors
• Second try/except Exception handles all other exceptions during client initialization
• Runtime ImportErrors now propagate correctly with their original message

Test plan

• Added test_google_provider_runtime_import_error_propagates - verifies socksio ImportError propagates correctly
• Added test_vertexai_provider_runtime_import_error_propagates - same for deprecated vertexai provider
• Added test_generative_ai_provider_runtime_import_error_propagates - same for deprecated generative-ai provider
• All tests pass: pytest tests/test_auto_client.py -v
• Linting passes: ruff check instructor/auto_client.py tests/test_auto_client.py

---

Important

Fixes runtime ImportError masking in from_provider for Google-related providers, ensuring correct error propagation.

• Behavior:
  • Fixes issue where from_provider masked runtime ImportError as ConfigurationError for Google, VertexAI, and Generative-AI providers.
  • Separates import error handling from client initialization in from_provider.
  • Runtime ImportError now propagates with original message.
• Tests:
  • Adds test_google_provider_runtime_import_error_propagates to verify socksio ImportError propagation.
  • Adds test_vertexai_provider_runtime_import_error_propagates for deprecated VertexAI provider.
  • Adds test_generative_ai_provider_runtime_import_error_propagates for deprecated Generative-AI provider.

^{This description was created by for 44de2f4. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to 44de2f4 in 2 minutes and 6 seconds. Click for details.

• Reviewed 241 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 4 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. instructor/auto_client.py:395

• Draft comment:
  Good separation: the try block for importing google.genai (and similarly for vertexai and generative-ai) now only catches genuine ImportErrors, allowing runtime ImportErrors (e.g. missing socksio) to propagate. This fixes the masking issue.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 85% This comment is purely informative, praising the separation of try blocks for imports. It doesn't provide any actionable feedback or suggestions for improvement. According to the rules, purely informative comments should be removed.

2. instructor/auto_client.py:795

• Draft comment:
  Similar improvements are applied to the vertexai and generative-ai branches. Consider refactoring the duplicated pattern (import block and corresponding error message) into a helper function to follow DRY principles.
• Reason this comment was not posted:
  Confidence changes required: 80% <= threshold 85% None

3. tests/test_auto_client.py:462

• Draft comment:
  The new regression tests for runtime ImportError propagation (for google, vertexai, and generative-ai) are well written, verifying that the error message contains 'socksio' and not the misleading 'google-genai' message. In the future consider abstracting common mocking logic to reduce duplication.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 40% vs. threshold = 85% This comment is about code quality and the DRY principle, which is explicitly mentioned in the additional rules as something to check for. The three test functions are indeed very similar and could potentially be refactored to reduce duplication. However, I need to consider: 1) Is this comment about NEW code in the diff? Yes, all three tests are new additions. 2) Is the suggestion actionable and clear? The comment suggests abstracting common logic but doesn't specify exactly how, which makes it somewhat vague. 3) Is this an important enough issue? The tests are regression tests that are quite readable as-is, and the duplication is limited to test setup code. 4) The rules say "Comments that suggest code quality refactors are good! But only if they are actionable and clear." This comment is somewhat actionable but not very specific about the implementation. While the comment correctly identifies duplication, it's not very specific about how to refactor. Test code duplication is often acceptable for clarity and independence. The comment doesn't provide a concrete suggestion for how to abstract the logic, making it less actionable. Additionally, these are regression tests where clarity and explicitness might be more valuable than DRY. The comment does align with the DRY principle from the rules, and the duplication is substantial enough that refactoring could improve maintainability. A pytest fixture or parametrized test could reduce the duplication. However, the lack of a specific, actionable suggestion and the fact that test code clarity is often prioritized over DRY makes this a borderline case. This comment identifies valid duplication but lacks specific guidance on how to refactor. Given that test code often prioritizes clarity over DRY, and the comment doesn't provide a concrete implementation suggestion, it falls short of being clearly actionable. I'll give it a moderate score reflecting uncertainty.

4. tests/test_auto_client.py:572

• Draft comment:
  It appears that in test_generative_ai_provider_runtime_import_error_propagates, patch.dict is passed a string "sys.modules" instead of using the actual sys.modules object. This might be a typographical error and could lead to unexpected behavior during patching.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 85% vs. threshold = 85% The comment points out that line 572 uses patch.dict("sys.modules", ...) (string) while the other two similar tests use patch.dict(sys.modules, ...) (object). Looking at the code, this is indeed the case. The question is: does patch.dict accept a string? According to Python's unittest.mock documentation, patch.dict can accept either a dictionary object or a string that will be imported. So technically both forms should work. However, the inconsistency is notable - if the string form works, why did the other two tests use the object form? This suggests it might be unintentional. The comment says it "might be a typographical error and could lead to unexpected behavior" - this is a reasonable concern given the inconsistency. This is about code that was changed in the diff, so it's relevant. While patch.dict can technically accept a string that will be imported, the inconsistency might be intentional for testing purposes, or it might not matter in practice. The comment uses hedging language like "might be" and "could lead to", which suggests uncertainty. However, the inconsistency is real and worth addressing for code consistency. The inconsistency is clear and verifiable from the diff. Even if both forms technically work, having inconsistent patterns in similar tests within the same file is a code quality issue. The comment is actionable - the author should make the three tests consistent. This is a legitimate code quality concern. This comment correctly identifies an inconsistency in the code where one test uses patch.dict("sys.modules", ...) while two similar tests use patch.dict(sys.modules, ...). This is a valid code quality issue that should be addressed for consistency, even if both forms technically work.

Workflow ID: wflow_2tY7uiWruzU6f1xV

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}