Repository navigation
fix: make ShiprocketQuickProvider fail soft on every error, not just 422 - #84
Merged
Merged
Conversation
Production incident: clicking "Location set — update" at checkout threw "Failed to fetch". Traced it to POST /order/delivery-quote returning a raw 503 from Fly's edge (confirmed via the browser's network tab), while every other endpoint responded normally in the same session. Root cause: ShiprocketQuickProvider (its own docstring: "UNVERIFIED PROVIDER... a best-effort guess", pending real vendor credentials per SPEC_DELIVERY_DISPATCH.md) only converted a 422 response into DeliveryUnavailableError. Every other failure mode — connection error, timeout, an unexpected HTTP status via raise_for_status(), a malformed or short response body — was a raw, uncaught exception that crashed the request handler. DeliveryService.get_quote()/request_delivery() and their callers already only catch DeliveryUnavailableError and fail soft correctly (per existing design); the gap was entirely inside this provider not honoring its own interface contract. Routed every vendor call through a shared _request()/_json_body() pair that converts requests.exceptions.RequestException, an unexpected HTTP status, a malformed JSON body, or a missing expected field into DeliveryUnavailableError — so a checkout shopper now sees "Delivery unavailable for this address" (400) instead of a crash, and request_delivery() falls back to a `failed` Delivery row as already designed, until Shiprocket Quick is wired up for real. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014t1eBFvEm5tiPyqQiQ3DNM
3 tasks done
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
Production bug report: clicking "Location set — update" at checkout threw
"Failed to fetch". Reproduced it live againstgroceror.store(Claude in Chrome, logged-in session) — the browser's Network tab showedPOST /order/delivery-quotecoming back as a raw503from Fly's edge, while every other endpoint (GET /inventory/browse, etc.) responded normally in the same session. A plaincurlto the same path got a clean401(unauthenticated), confirming the app itself was up — only this specific authenticated call was crashing.Root cause:
ShiprocketQuickProvider(engine/delivery/shiprocket_quick.py) is explicitly flagged in its own docstring as an unverified vendor integration — "a best-effort guess at a typical REST courier API," pending real Shiprocket Quick credentials perSPEC_DELIVERY_DISPATCH.md. It only converted a422response intoDeliveryUnavailableError. Every other failure mode — connection error, timeout, an unexpected HTTP status viaraise_for_status(), a malformed/short JSON body — was a raw, uncaughtrequestsexception that crashed the request handler, which Fly's edge then reports as a503with no CORS headers, which the browser then surfaces to JS as a generic"Failed to fetch".DeliveryService.get_quote()/request_delivery()and their callers already only catchDeliveryUnavailableErrorand fail soft correctly — the gap was entirely inside the provider not honoring its ownDeliveryProviderinterface contract.Fix
Routed every vendor call (
get_quote,create_delivery,get_status,cancel) through a shared_request()/_json_body()pair that convertsrequests.exceptions.RequestException, an unexpected HTTP status, a malformed JSON body, or a missing expected field intoDeliveryUnavailableError. A checkout shopper now sees a clean400("Delivery unavailable for this address") instead of a crash, andrequest_delivery()falls back to afailedDeliveryrow as already designed — this doesn't fix the underlying unverified-vendor problem (still blocked on real credentials), it just stops it from taking the whole request down.Test plan
tests/unit/test_shiprocket_quick_provider.py— 14 tests covering connection errors, timeouts, 422/404/409/5xx statuses, malformed/short response bodies, and the success paths for all four provider methodspytest tests/unit/test_shiprocket_quick_provider.py tests/unit/test_delivery_provider.py tests/unit/test_delivery_service.py tests/integration/test_delivery_dispatch.py -v— 42/42 passpytest tests/ -v— 624/624 pass🤖 Generated with Claude Code
https://claude.ai/code/session_014t1eBFvEm5tiPyqQiQ3DNM