fix(storage3): parse the real API error wire format - #1606
Open
grdsdev wants to merge 1 commit into
Open
Conversation
parse_api_error validated the response body against StorageApiError itself (message/code/status), but the API sends statusCode/error/message. Every API error therefore fell into the fallback and surfaced as code=InternalError, status=400, with the raw JSON in the message -- a 404 was indistinguishable from a 500. Parse into StorageApiErrorMessage, which already described the wire shape but was unused. The fallback now reports the response status instead of a hardcoded 400, and decodes with errors=replace so a non-utf-8 body cannot raise from the error path.
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
1 participant
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.
Follow-up to #1577. Neither fix from that PR is needed on
v3— pydantic'sValidationErroralready covers every malformed body shape, andexists()already branches on status rather than on an exception type. But verifying that surfaced a separate bug in the same code.The bug
parse_api_errorvalidated the response body againstStorageApiErroritself, whose fields aremessage/code/status. The API sendsstatusCode/error/message. No aliases are configured, so validation always failed and every API error fell into the fallback:{"statusCode":"404","error":"not_found","message":"Object not found"}code='InternalError' status=400code='not_found' status='404'<html>502 Bad Gateway</html>code='InternalError' status=400code='InternalError' status=502A 404 was indistinguishable from a 500, and the real message was buried in
Unable to parse error message: {raw json}. This affects all 9parse_api_errorcall sites acrossfile_api.pyandvectors.py.Two things suggest an oversight rather than a deliberate shape change:
VectorBucketErrorMessagesat in the same file describing the correct wire shape, imported nowhere.pytest.raises(StorageApiError), never.codeor.status— so the suite could not see it. Their mock body is already in the correct wire format.Changes
StorageApiErrorMessage(the renamed, previously-dead model), then buildStorageApiErrorfrom it.VectorBucketErrorMessageis kept as an alias — the vector endpoints return the same body.response.statusinstead of a hardcoded400.errors="replace", so a non-utf-8 body cannot raise from the error path itself.tests/test_exceptions.py: 9 unit tests, no infra needed. All 9 fail on the currentv3code and pass with this change.test_client_info_with_errortests to assert.message,.codeand.status.Verification
ruff check,ruff formatandmypyclean; the 9 new unit tests pass. Docker is not available in my environment, so the integration suite was not run locally — CI covers it.