Repository navigation
Conversation
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.
Problem
HTTP media type names are case-insensitive (RFC 9110 §8.3.1), but the response parser checks
content_type.endswith("json")case-sensitively.Through the public
simple.price.get()resource, a validContent-Type: Application/JSONsuccess response returns a raw string in loose mode, causingprice["bitcoin"].usdto raiseTypeError. Strict mode instead raisesAPIResponseValidationError. Lowercaseapplication/jsonworks. This affects dictionary/list response types that do not use the BaseModel JSON fallback. I do not claim CoinGecko's live API currently emits this casing.Change
Normalize casing only for the existing JSON suffix comparison. Preserve the existing content-type acceptance scope and fallback behavior. Add 32 sync/async dictionary/list regression cases across loose/strict validation, lowercase/mixed/uppercase types, charset parameters, and a mixed-case JSON suffix; document case-insensitive parsing.
This is independent of the environment-copy fix in the separate open PR; it branches directly from current
main(83af7a13bd615e4ee87c1443aa53a6baf34bdcff) and does not modify client configuration. The complete authenticated open+closed issue/PR inventory (53 entries) and relevant prior review/comments/timeline did not identify a duplicate or ownership claim. CONTRIBUTING.md explicitly permits manual patches that persist across generation. No generator config was invented.Verification
Windows; supported Python 3.11.5, httpx 0.28.1, pydantic 2.14.3, pytest 9.2.0:
simple.price.get()calls usinghttpx.MockTransportreproduced the consumerTypeErrorin loose mode and content-type validation error in strict mode for both mixed/uppercase JSON headers. Lowercase controls succeeded.python -m pytest tests/test_response.py -n 0: 58 passed.python -m ruff check src/coingecko_sdk/_response.py tests/test_response.py: passed.python -m ruff format tests/test_response.py: unchanged.usd=1.0; all 12 HTTP 400 combinations preservedBadRequestErrorand its dictionary error body. Error responses were not affected by this bug before the change either.text/plain, sync and async.All HTTP responses were supplied offline; no real API key, network API call, RPC, or funds were used. No LSP is configured in this environment; source references/callers were checked by text search instead.
The repository pins development Python 3.9.18. This run used a lightweight supported-3.11 venv rather than the pinned development lock (the lock's
backports-asyncio-runner==1.2.0requires Python <3.11). The pinned-3.9 environment, Pydantic v1 matrix, full client/Prism-backed resource suite, and full typecheck were not run. Temporary local dependencies were removed after verification.Risk / compatibility
No public symbols, signatures, client settings, or non-JSON fallback logic change. Already-supported JSON suffixes are compared case-insensitively; malformed/non-JSON content is not newly treated as JSON otherwise.
AI-assisted investigation and implementation; all runtime/test results above were actually exercised locally.