Report OpenAI failures instead of a bare 500, and keep the ingest worker alive - #11
Merged
Conversation
…ker alive An exhausted OPENAI_API_KEY quota made `POST /search` return `Internal Server Error` with the openai.RateLimitError only in the service logs, which reads as a bug in the graph query. Every retrieval endpoint embeds its query before it can search, so it is the first place an unusable key surfaces — /healthcheck and /episodes touch OpenAI not at all, and /messages only in the background. Map openai.APIError app-wide: 429 for a rate limit or exhausted quota, 502 when the key itself is rejected, 504 when OpenAI is unreachable. Registered on the base class so a subclass the SDK adds later is still mapped. 502 rather than echoing 401, because the caller's GRAPHITI_API_KEY was accepted and the credential to go fix is one they don't hold. The same quota error also killed ingestion for the life of the process. The worker loop caught only CancelledError, so any other exception escaped `while True` and ended the task — and nothing awaits it, so the traceback went nowhere while /messages carried on answering 202 into a queue with no reader. Catch Exception, log it, and take the next job. No retry: add_episode is not idempotent, and a dead key would be retried forever. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
main.py imports openai directly to map its exceptions onto HTTP statuses, but the package only arrived transitively through graphiti-core. Declare it. Also drop _drain's unused jobs parameter, whose docstring promised a bound it never applied, and lift a fixture-local import to the module top. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
main.py is the service's wiring; the status-and-message table for OpenAI failures is a policy with its own reasons, so it lives in graph_service/openai_errors.py alongside a note on which openai exceptions deliberately stay 500s. The handler stays in main.py, where the registration is. Tests: cover the two fallback branches (an error OpenAI returned, and one where it never answered), and build the openai errors per case rather than sharing one instance across cases, since raising the same instance twice accumulates state on it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
An exhausted OpenAI quota made this service look broken:
POST /searchanswered a bare500 Internal Server Error, and the ingestion worker died on the first failing job — silently,since nothing awaits its task — so
POST /messageskept answering 202 into a queue with noreader. Both read as bugs in the graph query; the real cause was only in the logs, as a traceback.
Changes
graph_service/openai_errors.pymaps anopenai.APIErrorto a status and a message namingwhat to go and fix: 429 for a rate limit or exhausted quota, 502 when OpenAI rejected this
service's credentials (not 401/403 — the caller's
GRAPHITI_API_KEYwas fine), 504 when OpenAIwas never reached. Registered in
main.pyon the base class, so a subclass the SDK adds lateris still mapped.
AsyncWorker.workerdrops a failing episode and keeps going instead of ending ingestion for thelife of the process. No retry:
add_episodeisn't idempotent and a dead key would retry forever.openaiis declared as a direct dependency, sincemain.pyimports it."success": truemeans queued, not ingested; and which endpoint fails first whenOPENAI_API_KEYis unusable.Testing
tests/test_upstream_failures.py— needs no OpenAI key and no database, so it runs in the defaultmake test. Covers each mapped status, plus that the worker survives a failing job and logs it.make lintclean, 33 tests passing.Flagged for a reviewer, not changed
Upstream said: ...). Deliberate — the caller here isthe operator who set the key, and the message is the only place the rate-limit/quota distinction
survives — but it does pass upstream account state to any bearer-token holder.
openai.OpenAIErrorsiblings that aren'tAPIError(LengthFinishReasonError,ContentFilterFinishReasonError) still surface as 500s. They describe a response that arrivedrather than an upstream that refused us, so neither gateway status fits.
🤖 Generated with Claude Code