fix: replace raise e with bare raise and improve error propagation in getBatchDetailsIter#134
Open
Adichapati wants to merge 1 commit into
Open
Conversation
- Use exception chaining (raise ... from e) to preserve traceback context - Capture status_code before the body-parse error path - Let RequestQuotaExceededError propagate naturally instead of collecting it into the catch-all handler This makes tracebacks actionable (original exception chain is preserved) and avoids masking quota errors as generic Exception.
There was a problem hiding this comment.
Pull request overview
Improves exception handling in Handler.getBatchDetailsIter to avoid masking original tracebacks, and aligns None comparisons with PEP 8 style in getBatchDetails.
Changes:
- Replace
raise ewith bareraisewhen re-raising exceptions fromrequests.post. - Refactor HTTP status handling in
getBatchDetailsIter(capturestatus_code, special-case 429 quota errors). - Use
is Noneinstead of== Noneforbatch_sizedefaulting ingetBatchDetails.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+403
to
408
| except RequestQuotaExceededError: | ||
| raise | ||
| except Exception as e: | ||
| if status_code == 429 and not isinstance(e, RequestQuotaExceededError): | ||
| raise RequestQuotaExceededError() from e | ||
| return handler_utils.return_or_fail(raise_on_fail, e) |
Comment on lines
392
to
+395
| try: | ||
| response = requests.post(url, json=batch, headers=headers) | ||
| except Exception as e: | ||
| raise e | ||
| except Exception: | ||
| raise |
Comment on lines
399
to
+404
| try: | ||
| if response.status_code == 429: | ||
| if status_code == 429: | ||
| raise RequestQuotaExceededError() | ||
| response.raise_for_status() | ||
| except RequestQuotaExceededError: | ||
| raise |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes an exception chaining anti-pattern in
Handler.getBatchDetailsIterthat was masking original tracebacks.Changes
Replace
raise ewith bareraise(line 395)raise ecreates a new exception without preserving the original traceback,making debugging significantly harder. Bare
raisere-raises the activeexception while keeping the full chain intact.
Improve HTTP status handling
status_codebefore the status-check try block so that thevalue is available if a JSON parse error occurs inside the block.
except RequestQuotaExceededError: raiseclause soquota errors propagate naturally instead of being routed through the
generic
except Exceptionhandler.raise RequestQuotaExceededError() from e)for the unlikely case where a 429 arrives but is wrapped in a different
exception type.
Use
is Noneinstead of== NoneingetBatchDetails(line 235)PEP 8 style consistency -- the sibling method
getBatchDetailsIteralreadyused
is None.Impact
Verification
python3 -m py_compile ipinfo/handler.py # passesThis is an AI-generated contribution. See bot-usage-policy.