Repository navigation
Conversation
…/5xx ExceptionHandlers.exception read `error.errors` without calling it. For a pydantic ValidationError that is a bound method, not a list, so a required provider QueryParams field missing from the request fell through to "500 Unexpected Error -> ValidationError". It now calls errors(); only a missing QueryParams field is a 422, so a provider Data model failing on an upstream response that dropped a key stays a 500. The detail goes through jsonable_encoder, because the error input can hold dates. TimeoutError now maps to 504 and json.JSONDecodeError (an upstream that answered with HTML or an empty body) to 502, each with a clear detail. AppLoader registers both by type: the Exception catch-all runs outside the CORS middleware, so its responses carry no CORS headers and a browser sees a CORS failure instead of the status. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 9, 2026
…t input Provider extensions name query models freely (e.g. BalanceQuery), so the title check alone left their missing inputs at 500. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dlee-bq
requested changes
Oct 18, 2026
dlee-bq
left a comment
Contributor
There was a problem hiding this comment.
Hi @artcashin, thanks for the PR. This is good for the most part, but there's some handling needing further consideration:
- requests.Timeout isn’t a subclass of TimeoutError, and Nasdaq and others use requests, so those timeouts are still 500s.
- When amake_request gets a blocked page served as text/html, aiohttp raises ContentTypeError, not JSONDecodeError. That is probably the most common “non-JSON upstream” case, and it isn’t covered.
- Python 3.10: core declares >=3.10, but on 3.10 asyncio.TimeoutError is not the builtin TimeoutError. aiohttp timeouts would stay 500 there.
- The 422 body has a different shape: a single dict, with loc: ["code"] and pydantic’s url. FastAPI’s own 422s, and the existing validation path, return a list with loc: ["query", ...]. Clients that parse FastAPI-style 422s will see the difference, and only the first missing field is reported.
Do we need to register additional handling in the Python Interface's Exception Handlers?
This branch has not been deployed
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
ExceptionHandlers.exceptiongives REST clients the wrong status in three cases:QueryParamsfield is absent from the request,validationpasses theValidationErroron toexception. That handler readserror.errorswithout calling it. For a pydanticValidationErrorthat is a bound method, not a list, so its "missing" branch never runs and the client gets500 Unexpected Error -> ValidationError.Unexpected Error -> TimeoutError.json.JSONDecodeErroris aValueError, so it takes theValueErrorbranch and the client is told its own request was invalid, when the upstream sent HTML or an empty body.The
Exceptioncatch-all also runs in Starlette'sServerErrorMiddleware, outsideCORSMiddleware, so its responses carry no CORS headers. A browser client sees a CORS failure instead of the status.Change
exceptioncallserrors()when it is callable. The 422 "missing" branch now applies only when the error's title containsQueryParams. Witherrors()called, a providerDatamodel failing on an upstream response that dropped a key would otherwise become a 422 that blames the client; it stays a 500. The detail goes throughjsonable_encoder, because the error'sinputcan hold dates thatJSONResponsecannot serialize.TimeoutErrormaps to 504, with the error's message or "The upstream data source did not respond in time."json.JSONDecodeErrormaps to 502: "The data source returned a response that is not valid JSON."AppLoader.add_exception_handlersregistersTimeoutErrorandjson.JSONDecodeErrorby type. Handlers registered by type run inside the CORS middleware, so these responses keep their CORS headers.Tests
New in
tests/api/test_exception_handlers.py:QueryParamsfield with a date in the input gives a 422 with a JSON-encoded detail;Datafield stays a 500;TimeoutErrorgives a 504, with and without a message;JSONDecodeErrorgives a 502.New in
tests/api/test_app_loader.py: behindCORSMiddleware, the 504 and 502 responses carryaccess-control-allow-origin(parametrized, two cases).Five of these fail without the change. The
Data-field test passes either way; it checks that the narrowed 422 branch keeps that case a 500.test_exception_handler_iterable_missingnow gives its fake error aQueryParamstitle, because the 422 branch requires one; its assertion is unchanged. The two files pass 25/25, andopenbb_platform/core/testspasses 2,254 with 16 skipped (Python 3.12).Results from a deployment
OpenBQ's Docker image has run these two files as a build-time patch over openbb-core 2.0.1, which matches
developfor both files.500 Unexpected Error -> ValidationError, out of 100 500s for that class of request. With the patch, a request without a required provider input answers 422 with{"type": "missing", "loc": ["code"], ...}. We checked this on/cftc/cot, withstart_datein the request so the error's input holds a date.Unexpected Error -> TimeoutError. They now answer 504.rest_apiapp. It passes 11/11; on unpatched 2.0.1 it had 7 failures and 1 error. The module also covers fix(core): mark required provider-only query params required in the widget #7752 and feat(core): optional per-provider deadline for commands #7753.Not included
ValueErrors still map to 422, as before.🤖 Generated with Claude Code