fix(cdk): improve rate limiting error messages to suggest reducing num_workers#949
fix(cdk): improve rate limiting error messages to suggest reducing num_workers#949sophiecuiy wants to merge 4 commits intomainfrom
Conversation
…ncurrency or workers Co-Authored-By: sophie.cui@airbyte.io <sophie.cui@airbyte.io>
🤖 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 TricksTesting This CDK VersionYou can test this version of the CDK using the following: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@devin/1773181449-improve-rate-limit-error-messaging#egg=airbyte-python-cdk[dev]' --help
# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch devin/1773181449-improve-rate-limit-error-messagingPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
Co-Authored-By: sophie.cui@airbyte.io <sophie.cui@airbyte.io>
…P status code Co-Authored-By: sophie.cui@airbyte.io <sophie.cui@airbyte.io>
devin-ai-integration
bot
changed the title
There was a problem hiding this comment.
Pull request overview
Updates CDK rate limiting messaging so users hitting HTTP 429 get actionable guidance to reduce num_workers, and adds a rate-limit-specific failure path when rate-limit retries are exhausted.
Changes:
- Updated the default 429 error mapping message to include explicit “reduce number of workers” guidance.
- Added a dedicated
RateLimitBackoffExceptionhandling branch inHttpClient._send_with_retrywith a rate-limit-specific user message, and simplified the generic retry-exhaustion message. - Updated rate-limit retry logging to explicitly mention rate limiting and recommend reducing workers.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
airbyte_cdk/sources/streams/http/error_handlers/default_error_mapping.py |
Updates the default 429 mapped message to include actionable worker guidance. |
airbyte_cdk/sources/streams/http/http_client.py |
Adds rate-limit-specific traced exception messaging when RateLimitBackoffException exhausts and adjusts generic exhaustion text. |
airbyte_cdk/sources/streams/http/rate_limiting.py |
Changes backoff retry log line to explicitly call out rate limits and num_workers guidance. |
unit_tests/sources/streams/http/test_http.py |
Updates assertions to match the new 429 message. |
unit_tests/sources/declarative/requesters/error_handlers/test_http_response_filter.py |
Updates expected 429 error message string for declarative error handling tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| logger.info( | ||
| f"Caught retryable error '{str(exc)}' after {details['tries']} tries. Waiting {details['wait']} seconds then retrying..." | ||
| f"Rate limit hit after {details['tries']} tries. Waiting {details['wait']} seconds then retrying. " | ||
| f"Try decreasing the number of workers to stay within API rate limits." |
There was a problem hiding this comment.
The new retry log message no longer includes the underlying exception text (previously str(exc)), which can remove useful context like the request URL / error details when diagnosing repeated 429s. Consider appending str(exc) (or key fields like request URL) to this log line while keeping the actionable guidance.
| f"Try decreasing the number of workers to stay within API rate limits." | |
| f"Try decreasing the number of workers to stay within API rate limits. " | |
| f"Last error: {str(exc)}" |
There was a problem hiding this comment.
Good catch — restored str(exc) as Last error: {str(exc)} in the log line so debug context is preserved alongside the new guidance. Fixed in 7301186.
| with pytest.raises( | ||
| AirbyteTracedException, | ||
| match="Exhausted available request attempts. Please see logs for more details. Exception: HTTP Status Code: 429. Error: Too many requests.", | ||
| match="Rate limit exceeded \\(HTTP status code 429\\). Try decreasing the number of workers to stay within API rate limits.", | ||
| ): | ||
| stream.exit_on_rate_limit = True | ||
| list(stream.read_records(SyncMode.full_refresh)) |
There was a problem hiding this comment.
This assertion used to verify the full "Exhausted available request attempts..." message (including the removed "Please see logs for more details." text). With the updated regex only matching the 429 mapping string, the change in HttpClient._send_with_retry’s generic exhaustion message is no longer covered by tests. Consider adding/adjusting an assertion to explicitly check the new exhaustion message (or at least ensure the old "Please see logs" text is absent).
There was a problem hiding this comment.
Good point — updated the test to assert the full "Exhausted available request attempts. Exception: Rate limit exceeded (HTTP status code 429). Try decreasing the number of workers to stay within API rate limits." message, which verifies both the generic exhaustion prefix and the new 429 mapping text. Fixed in 7301186.
…erage per review Co-Authored-By: sophie.cui@airbyte.io <sophie.cui@airbyte.io>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR improves HTTP 429 rate limit error messaging and handling across the Airyte CDK HTTP client stack. It replaces generic rate limit messages with more descriptive guidance suggesting users reduce worker count, implements specialized exception handling for rate limit errors, and updates corresponding test expectations. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
fix(cdk): improve rate limiting error messages to suggest reducing num_workers
Summary
Improves error messaging across the CDK's HTTP rate limiting pipeline so that users encountering 429 errors receive actionable guidance to reduce the number of workers.
Three source files changed:
default_error_mapping.py— Updated the default 429 error message from"HTTP Status Code: 429. Error: Too many requests."to"Rate limit exceeded (HTTP status code 429). Try decreasing the number of workers to stay within API rate limits."http_client.py— Added a dedicatedRateLimitBackoffExceptionbranch in_send_with_retrythat surfaces a rate-limit-specific message when retries are exhausted (applies whenexit_on_rate_limit=False, i.e. the "retry endlessly" path). Also removed"Please see logs for more details."from the generic exhaustion message.rate_limiting.py— Updated the retry log message inrate_limit_default_backoff_handlerto explicitly mention rate limiting and the num_workers guidance, while preserving the original exception text (Last error: {str(exc)}) for debug context.Updates since last revision
"Rate limit exceeded (HTTP status code 429). ..."instead of dropping the status code entirely).num_workersis the standard user-configurable field name).str(exc)in the rate-limit retry log line so underlying error details are preserved alongside the new guidance.exit_on_rate_limit=Truetest assertion to verify the full message including the"Exhausted available request attempts. Exception: ..."prefix, ensuring both the generic exhaustion wrapper and the new 429 mapping text are covered by tests.Review & Testing Checklist for Human
"HTTP Status Code: 429. Error: Too many requests."). Thedefault_error_mappingchange affects all connectors using the default error handler. Search theairbytemonorepo for any code that matches against this exact string._send_with_retrynow says"Exhausted available request attempts. Exception: {e}"instead of"Exhausted available request attempts. Please see logs for more details. Exception: {e}". This affects ALL non-rate-limit backoff failures — confirm this is acceptable.exit_on_rate_limit=Truepath: Whenexit_on_rate_limit=True, 429s follow theDefaultBackoffExceptionpath (notRateLimitBackoffException), so the new dedicated branch in_send_with_retrydoes NOT fire. The improved message still reaches the user via the exception string from the updated 429 mapping, but the user-facingmessagefield onAirbyteTracedExceptionwill show the generic exhaustion text wrapping the new 429 text. The test now verifies this full combined string.num_workersas an example config key; verify this is the standard term users would recognize.Suggested manual test plan: Trigger a 429 response in a connector using the default error handler (e.g., source-pokeapi with aggressive concurrency) and confirm the new error message appears in both logs and the traced exception output.
Notes
Requested by sophiecuiy
Devin session
Summary by CodeRabbit
Release Notes