Repository navigation
fix(grpc): surface QdrantClientResourceExhaustedError with full retry-after - #175
Open
jackiectl2 wants to merge 1 commit into
Open
jackiectl2 wants to merge 1 commit into
jackiectl2 wants to merge 1 commit into
Conversation
…-after
The gRPC ResourceExhausted interceptor never delivered a usable
QdrantClientResourceExhaustedError:
- `error.metadata` is a `Headers` object, so `get('retry-after')?.[0]`
took only the first character of the value ("15" became 1).
- The error set its own `name`, so connect no longer recognized it as a
ConnectError and wrapped it into a new `Code.Unknown` ConnectError
(the real error was only reachable via `cause`).
- It inherited ConnectError's `Symbol.hasInstance`, so
`err instanceof QdrantClientResourceExhaustedError` was true for any
ConnectError.
Read the full header value, keep the base `ConnectError` name so connect
passes the error through, and restore the prototype chain `instanceof`
check. Add unit tests against an in-process connect server.
Signed-off-by: Tianlang (Jackie) Chen <305351710+jackiectl2@users.noreply.github.com>
Author
|
The failing check is the |
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.
What
The gRPC client's ResourceExhausted handling (added in #95) never reaches users in a usable form:
error.metadatais aHeadersobject, soerror.metadata.get('retry-after')?.[0]keeps only the first character of the value ("15"→retry_after === 1).QdrantClientResourceExhaustedErroroverridesname, so connect'sConnectError[Symbol.hasInstance]no longer recognizes it andConnectError.from()wraps it in a newCode.Unknownerror. Callers get[unknown] [resource_exhausted] ...and the real error only viacause.Symbol.hasInstance,err instanceof QdrantClientResourceExhaustedErroristruefor anyConnectError.Changes
retry-aftervalue.ConnectErrorname so connect passes the error through unchanged (codestaysResourceExhausted).instanceofcheck on the subclass.The REST client has the same
?.[0]pattern; that line is already changed in #128, so this PR only touches the gRPC package.Testing
packages/js-client-grpc/tests/unit/api-client.test.tsrunscreateApisagainst an in-process connect server (no Qdrant needed): ResourceExhausted withretry-after: 15now yieldsQdrantClientResourceExhaustedErrorwithcode8 andretry_after15, and a NotFound error is not an instance of it. Both tests fail without the fix.pnpm -r pre-checkpasses locally (Node 22). Integration tests were not run locally (they need a Qdrant server).I did not bump versions or CHANGELOG since recent fix PRs leave that to the release PR; happy to add an entry if preferred.