Skip to content

Inject fetch into REST transport#128

Open
Inv1x wants to merge 2 commits into
qdrant:masterfrom
Inv1x:feature/fetch-injection-rest-transport
Open

Inject fetch into REST transport#128
Inv1x wants to merge 2 commits into
qdrant:masterfrom
Inv1x:feature/fetch-injection-rest-transport

Conversation

@Inv1x
Copy link
Copy Markdown

@Inv1x Inv1x commented Apr 1, 2026

Summary

  • Replace the REST transport's hard dependency on undici with a local fetch wrapper.
  • Add support for injecting a custom fetch implementation when creating QdrantClient.
  • Remove dispatcher-based Node-specific wiring and delegate transport behavior to the host runtime.
  • Update documentation and unit coverage for the new fetch injection path.

Testing

  • Added a unit test that verifies an injected fetch is used instead of global.fetch.
  • Added a unit assertion that request init no longer includes dispatcher.

- Remove undici dispatcher dependency
- Delegate transport setup to host runtime
- Add tests for custom fetch injection
Copilot AI review requested due to automatic review settings April 1, 2026 15:34
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the REST client’s hard dependency on undici/dispatcher wiring by introducing a local fetch-based transport and allowing callers to inject a custom fetch implementation via QdrantClient construction.

Changes:

  • Remove undici + dispatcher-based RequestInit wiring and dependency entries.
  • Add a new local fetcher abstraction and plumb fetch injection through QdrantClient → REST transport.
  • Update docs and unit tests to cover fetch injection and ensure dispatcher is no longer set.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
pnpm-lock.yaml Removes top-level undici lock entries.
packages/js-client-rest/package.json Drops undici dependency.
packages/js-client-rest/src/dispatcher.ts Deletes Node-specific dispatcher implementation.
packages/js-client-rest/src/fetcher.ts Adds new fetch wrapper used by REST transport (core behavioral change).
packages/js-client-rest/src/api-client.ts Switches transport construction to the new fetcher + supports injected fetch.
packages/js-client-rest/src/types.ts Adds fetch to internal REST args.
packages/js-client-rest/src/qdrant-client.ts Exposes fetch injection on QdrantClientParams; deprecates maxConnections behavior.
packages/js-client-rest/tests/unit/api-client.test.ts Adds assertions for no dispatcher and verifies injected fetch is used.
packages/js-client-rest/README.md Updates support section and documents fetch injection.
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/js-client-rest/src/api-client.ts
Comment thread packages/js-client-rest/src/fetcher.ts
Comment thread packages/js-client-rest/tests/unit/api-client.test.ts
Comment thread packages/js-client-rest/README.md
Comment thread packages/js-client-rest/src/fetcher.ts Outdated
- Preserve full Retry-After header on 429 errors
- Keep FormData request bodies intact
- Parse large JSON integers as bigint when supported
giancarloerra pushed a commit to giancarloerra/SocratiCode that referenced this pull request May 12, 2026
Under Node 26+, the very first qdrant request crashes with
`UND_ERR_INVALID_ARG: invalid onError method`. Root cause is a version
mismatch: @qdrant/js-client-rest constructs an undici.Agent from its
pinned undici ^6 and passes it as the dispatcher to Node's built-in
fetch(), which under Node 26 uses a newer undici with stricter
dispatcher-hook validation.

The bug surfaces on the first real codebase_search / codebase_index
call — the MCP handshake succeeds, then everything fails. The error
message gives no hint about Node version, so users on Node 26+ lose
significant time debugging.

This change:
- Adds a runtime pre-flight check at index.ts entry that prints a
  clear actionable error and exits 1. Per ESM the imports below
  evaluate first, but qdrant-js's module init is side-effect-light,
  so exiting at the first top-level statement is enough.
- Tightens engines.node to `>=18.0.0 <26.0.0` so npm/npx warns at
  install time.

Both can be reverted once one of qdrant/qdrant-js#123 (undici major
upgrade) or qdrant/qdrant-js#128 (inject fetch) lands.

Refs: qdrant/qdrant-js#134

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants