fix(ogc): report the service's own 403 reason, and name the URL - #374
Merged
Merged
Conversation
A 403 returned fixed text naming only "query exceeding server limits" and never read the body, so a missing, expired, or revoked API_USGS_PAT -- the most common real 403 against api.waterdata.usgs.gov -- was reported as a query-size problem while the service's own "API key revoked" was discarded. 403 now shares the generic path: the JSON error body when there is one (a USGS code/description envelope or a gateway message, flat or nested under "error"), the text snippet otherwise, and credential guidance only when there is no body to quote. No OGC error message named the URL. The legacy path appends it, this one never did, and nothing downstream restores it -- FanOutInterrupted builds its "Cause:" line from this same string -- so a fan-out that died on one bad chunk gave no indication of which request was in flight. Reading the URL is guarded: httpx raises on `.url` for a hand-built response, and an error path must not fail while reporting a failure. Not addressed here: _querying._raise_for_status and ogc.errors._raise_for_non_200 remain two implementations of one contract, and the 414 "split your query" remediation is still only on the legacy side. Unifying them means moving the message builder to a layer `ogc` may import (.importlinter forbids ogc -> _querying) -- a structural change rather than a bug fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BTaSm7HmVb94RSJiKW4WAS
thodson-usgs
marked this pull request as ready for review
August 14, 2026 15:16
thodson-usgs
force-pushed
the
fix/ogc-error-messages
branch
from
August 14, 2026 15:21
036d6ea to
765b374
Compare
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.
Two user-facing defects in the OGC error message
1. A 403 was a canned guess that never read the body
The 403 branch returned fixed text and explicitly did not consult the response — its docstring said so. But a missing, expired, or revoked
API_USGS_PATis the most common real 403 againstapi.waterdata.usgs.gov, and that user was told to go shrink their query while the service's own"API key revoked"was discarded. The legacy_queryingpath would have surfaced it.It now renders the JSON envelope like every other status, and when there is no envelope to quote, names both plausible causes instead of only the rarer one.
2. No OGC error message named the URL
The legacy path appends
(URL: ...); this one never did, and nothing downstream restores it —FanOutInterruptedbuilds itsCause:line from this same string. So a 32-chunk fan-out that died on one badparameter_coderaised a message with no indication of which chunk, which sites, or which values were in flight.Reading the URL is guarded:
httpxraises on.urlfor a hand-built response, and an error path must not fail while reporting a failure.The JSON-envelope read is factored into one helper so the 403 branch and the generic path cannot drift apart again.
Not addressed here
_querying._raise_for_statusandogc.errors._raise_for_non_200remain two implementations of one contract, and each is still missing the other's improvement — the 414 "split your query" remediation exists only on the legacy side. Unifying them means moving the message builder down to a layerogcis permitted to import (.importlinterforbidsogc -> _querying), which is a structural change rather than a bug fix and deserves its own review.Testing
mypy --strict,ruff,xenon,complexipy,lint-importsall pass.Found by the scan in #372, split out because it changes user-visible behavior. Independent of #372 and #373 — branches off main.
🤖 Generated with Claude Code
https://claude.ai/code/session_01BTaSm7HmVb94RSJiKW4WAS