fix(source-shopify): classify bulk job auth errors as config_error#81363
fix(source-shopify): classify bulk job auth errors as config_error#81363devin-ai-integration[bot] wants to merge 3 commits into
Conversation
… creation Shopify API authentication errors (e.g. 'Invalid API key or access token') during GraphQL BULK job creation were being classified as system_error via the catch-all BulkJobNonHandableError. This prevented the platform from signaling users to reconfigure their credentials. - Add BulkJobAuthFailedError exception with failure_type=config_error - Add _is_auth_error detection in _on_non_handable_job_error - Update BaseBulkException.__init__ to support user-facing message parameter - Add unit tests covering auth error classification Co-Authored-By: bot_apk <apk@cognition.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksPR Slash CommandsAirbyte Maintainers (that's you!) can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful Resources
|
Co-Authored-By: bot_apk <apk@cognition.ai>
Co-Authored-By: bot_apk <apk@cognition.ai>
|
Deploy preview for airbyte-docs ready!
Deployed with vercel-action |
|
|
↪️ Triggering Reason: Draft PR with CI passing (35/35). Classifies Shopify bulk job auth errors as config_error instead of system_error. Running prove-fix to validate. |
|
🧪 Fix Validation Evidence🟢 Outcome: Fix ProvenRegression tests confirm no behavioral regression across all 47 streams (1658 records, identical target vs control counts). The fix is an edge-case-only change that only triggers on invalid/expired credentials — no healthy connection can exercise this code path. Unit tests (3 new parametrized cases) directly validate the auth error classification logic. Code review confirms the fix follows the existing REST API 401 → 🚦 Next Steps
📋 Connector & PR DetailsConnector: 📝 Evidence PlanProving CriteriaThe fix only activates when Shopify returns an auth error. A connection with an invalid token should fail with Disproving Criteria
Testing StrategyRegression tests only — edge-case-only fix that cannot be triggered by any healthy connection. Unit tests are the primary evidence. Cases Attempted
✅ Pre-flight Checks
📊 Detailed Evidence LogRegression Test Workflow: https://github.com/airbytehq/airbyte-ops-mcp/actions/runs/28587395901
Unit Test Evidence: 3 new parametrized test cases in
All 272 connector unit tests pass. Note: Connection IDs and detailed logs are recorded in the linked private issue. |
|
|
|
↪️ Triggering Reason: /ai-prove-fix completed. CI passing (35/35). Ready for AI review. |
Reviewing PR for connector safety and quality.
|
🛡️ AI PR Review Report🟢 Review Action: APPROVEDAll 12 gates passed. 🔶 Risk Level: 3/5Error-handling logic change in source-shopify bulk job path; classifies auth errors as 📋 PR DetailsConnector(s): 🔍 Gate Evaluation DetailsGate-by-Gate Analysis
PR Hygiene Details:
Test Coverage Details:
Code Security Details:
Live / E2E Tests Details:
📚 Evidence ConsultedEvidence
|
|
↪️ Triggering Reason: AI review APPROVED. Prove-fix passed (no regression, 47 streams, 1658 records identical). CI passing (35/35). Ready for merge review. |
Auto-merge evaluation: FAILPreconditions (all must pass)
Change scope (at least one must pass)❌ No matching change scope detected. |
What
Resolves https://github.com/airbytehq/oncall/issues/13031:
Shopify API authentication errors (e.g.
"[API] Invalid API key or access token") during GraphQL BULK job creation were classified assystem_errorvia the catch-allBulkJobNonHandableError. This prevented the platform from signaling users to reconfigure their credentials.How
exceptions.py: AddedBulkJobAuthFailedErrorwithfailure_type = FailureType.config_error. UpdatedBaseBulkException.__init__to accept an optional user-facingmessageparameter (previously onlyinternal_messagewas forwarded toAirbyteTracedException).job.py: Added_is_auth_error()to detect auth-related keywords in the error list (both string and dict error formats). Modified_on_non_handable_job_error()to raiseBulkJobAuthFailedErrorwith a clear user-facing message when auth errors are detected; non-auth errors still raiseBulkJobNonHandableErrorwithsystem_error.Declarative-First Evaluation
Not applicable — this is a Python CDK connector (
cdk:pythontag), not a declarative/manifest-only connector.Review guide
source_shopify/shopify_graphql/bulk/exceptions.py— newBulkJobAuthFailedErrorclass, updatedBaseBulkException.__init__signaturesource_shopify/shopify_graphql/bulk/job.py—_is_auth_error()detection +_on_non_handable_job_error()branchingunit_tests/conftest.py— fixtures for auth error responses (server-level string and user-level dict)unit_tests/graphql_bulk/test_job.py— 3 parametrized test cases: auth error from server errors, auth error from user errors, non-auth error unchangedTest Coverage
Three new parametrized test cases in
test_auth_error_classified_as_config_error:auth_error_from_server_errors_string— server-level string error →config_errorauth_error_from_user_errors_dict— user-level dict error →config_errornon_auth_error_stays_system_error— non-auth error →system_error(regression guard)All 272 unit tests pass locally.
User Impact
Users with invalid or expired Shopify API tokens will now see a clear
config_errormessage ("Shopify access token is invalid or has expired.") instead of a generic "Something went wrong in the connector"system_error. This enables the platform to correctly prompt users to reconfigure their credentials.Can this PR be safely reverted and rolled back?
Link to Devin session: https://app.devin.ai/sessions/b8595a9402b347568442c6385768b48e