diff --git a/src/http/baseSettings.ts b/src/http/baseSettings.ts index 7dd2dd71..701128da 100644 --- a/src/http/baseSettings.ts +++ b/src/http/baseSettings.ts @@ -1,8 +1,9 @@ -import { Dispatcher, getGlobalDispatcher } from "undici"; +import { Dispatcher } from "undici"; // eslint-disable-next-line no-restricted-imports import packageJson from "../../package.json" with { type: "json" }; import * as os from "os"; import { TIMEOUT_SECS_DEFAULT } from "./apiCore.js"; +import { resolveDefaultDispatcher } from "./dispatcher.js"; export interface MindeeApiConstructorProps { apiKey?: string; @@ -25,7 +26,7 @@ export abstract class BaseSettings { } else { this.apiKey = apiKey; } - this.dispatcher = dispatcher ?? getGlobalDispatcher(); + this.dispatcher = dispatcher ?? resolveDefaultDispatcher(); this.hostname = this.hostnameFromEnv(); this.timeoutSecs = process.env.MINDEE_REQUEST_TIMEOUT ? parseInt(process.env.MINDEE_REQUEST_TIMEOUT) diff --git a/src/http/dispatcher.ts b/src/http/dispatcher.ts new file mode 100644 index 00000000..036d9777 --- /dev/null +++ b/src/http/dispatcher.ts @@ -0,0 +1,21 @@ +import { Agent, Dispatcher, getGlobalDispatcher } from "undici"; +import { logger } from "@/logger.js"; + +/** Library-owned fallback dispatcher, shared across all components. */ +let fallbackDispatcher: Dispatcher | undefined; + +/** + * Returns the global undici dispatcher, provided it was created by the same + * undici copy this library imports. + */ +export function resolveDefaultDispatcher(): Dispatcher { + const globalDispatcher = getGlobalDispatcher(); + if (globalDispatcher instanceof Dispatcher) { + return globalDispatcher; + } + logger.debug( + "Global dispatcher belongs to a different undici instance, using a library-owned Agent instead." + ); + fallbackDispatcher ??= new Agent(); + return fallbackDispatcher; +} diff --git a/src/http/index.ts b/src/http/index.ts index 5b475e22..fe107180 100644 --- a/src/http/index.ts +++ b/src/http/index.ts @@ -5,3 +5,4 @@ export { cleanRequestData, } from "@/v1/http/responseValidation.js"; export { BaseSettings } from "./baseSettings.js"; +export { resolveDefaultDispatcher } from "./dispatcher.js"; diff --git a/src/input/urlInput.ts b/src/input/urlInput.ts index 6c9f09af..97c71d56 100644 --- a/src/input/urlInput.ts +++ b/src/input/urlInput.ts @@ -3,9 +3,10 @@ import { URL } from "url"; import { basename, extname } from "path"; import { randomBytes } from "crypto"; import { writeFile } from "fs/promises"; -import { request, Dispatcher, getGlobalDispatcher } from "undici"; +import { request, Dispatcher } from "undici"; import { logger } from "@/logger.js"; import { MindeeInputSourceError } from "@/errors/index.js"; +import { resolveDefaultDispatcher } from "@/http/dispatcher.js"; import { BytesInput } from "./bytesInput.js"; /** Remote input source represented by a validated HTTPS URL. */ @@ -20,7 +21,7 @@ export class UrlInput extends InputSource { ) { super(); this.url = url; - this.dispatcher = dispatcher ?? getGlobalDispatcher(); + this.dispatcher = dispatcher ?? resolveDefaultDispatcher(); logger.debug("Initialized URL input source."); } diff --git a/tests/http/baseSettings.spec.ts b/tests/http/baseSettings.spec.ts new file mode 100644 index 00000000..59285b95 --- /dev/null +++ b/tests/http/baseSettings.spec.ts @@ -0,0 +1,71 @@ +import assert from "node:assert/strict"; +import { afterEach, describe, it } from "node:test"; +import { Agent, Dispatcher, getGlobalDispatcher, setGlobalDispatcher } from "undici"; +import { BaseSettings } from "@/http/baseSettings.js"; +import { UrlInput } from "@/input/index.js"; + +class TestSettings extends BaseSettings { + constructor(dispatcher?: Dispatcher) { + super("dummy-key", dispatcher); + } + protected apiKeyFromEnv(): string { + return "dummy-key"; + } + protected hostnameFromEnv(): string { + return "test-host"; + } +} + +describe("BaseSettings – dispatcher resolution", () => { + const originalDispatcher = getGlobalDispatcher(); + + afterEach(() => { + setGlobalDispatcher(originalDispatcher); + }); + + it("uses an explicitly provided dispatcher as-is", () => { + const agent = new Agent(); + const settings = new TestSettings(agent); + assert.strictEqual(settings.dispatcher, agent); + }); + + it("uses the global dispatcher when it comes from the same undici copy", () => { + const agent = new Agent(); + setGlobalDispatcher(agent); + const settings = new TestSettings(); + assert.strictEqual(settings.dispatcher, agent); + }); + + it("ignores a global dispatcher from a foreign undici instance", () => { + // Simulates Node's built-in fetch (or another undici copy) having + // registered its dispatcher on the shared global symbol: such objects are + // not instances of this library's undici Dispatcher class. + const foreignDispatcher = { dispatch: () => true } as unknown as Agent; + setGlobalDispatcher(foreignDispatcher); + const settings = new TestSettings(); + assert.notStrictEqual(settings.dispatcher, foreignDispatcher); + assert.ok(settings.dispatcher instanceof Dispatcher); + }); + + it("reuses the same fallback dispatcher across instances", () => { + const foreignDispatcher = { dispatch: () => true } as unknown as Agent; + setGlobalDispatcher(foreignDispatcher); + const first = new TestSettings(); + const second = new TestSettings(); + assert.strictEqual(first.dispatcher, second.dispatcher); + }); + + it("UrlInput ignores a foreign global dispatcher as well", () => { + const foreignDispatcher = { dispatch: () => true } as unknown as Agent; + setGlobalDispatcher(foreignDispatcher); + const input = new UrlInput({ url: "https://example.com/file.pdf" }); + assert.notStrictEqual(input.dispatcher, foreignDispatcher); + assert.ok(input.dispatcher instanceof Dispatcher); + }); + + it("UrlInput uses an explicitly provided dispatcher as-is", () => { + const agent = new Agent(); + const input = new UrlInput({ url: "https://example.com/file.pdf", dispatcher: agent }); + assert.strictEqual(input.dispatcher, agent); + }); +}); diff --git a/tests/v2/client/foreignDispatcher.integration.ts b/tests/v2/client/foreignDispatcher.integration.ts new file mode 100644 index 00000000..63c9a22c --- /dev/null +++ b/tests/v2/client/foreignDispatcher.integration.ts @@ -0,0 +1,98 @@ +import { before, describe, it } from "node:test"; +import assert from "node:assert/strict"; + +// IMPORTANT: this file must NOT statically import "undici" or the SDK. +// This test showcases how the library previously behaved and broke when reusing a foreign global dispatcher. + +describe( + "MindeeV2 – Integration – foreign global dispatcher", + { timeout: 120000 }, + () => { + let apiKey: string; + let modelId: string; + let pdfBuffer: Buffer; + let undici: typeof import("undici"); + let mindee: typeof import("@/index.js"); + let mindeeHttp: typeof import("@/v2/http/index.js"); + let foreignDispatcher: import("undici").Dispatcher; + + before(async () => { + apiKey = process.env["MINDEE_V2_API_KEY"] ?? ""; + modelId = process.env["MINDEE_V2_SE_TESTS_FINDOC_MODEL_ID"] ?? ""; + const pdfUrl = process.env["MINDEE_V2_SE_TESTS_BLANK_PDF_URL"]; + assert.ok( + pdfUrl, + "MINDEE_V2_SE_TESTS_BLANK_PDF_URL must be set to run this integration test", + ); + + const response = await fetch(pdfUrl); + assert.ok(response.ok, `Failed to download the test PDF from: ${pdfUrl}`); + pdfBuffer = Buffer.from(await response.arrayBuffer()); + + undici = await import("undici"); + mindee = await import("@/index.js"); + mindeeHttp = await import("@/v2/http/index.js"); + + foreignDispatcher = undici.getGlobalDispatcher(); + assert.ok( + !(foreignDispatcher instanceof undici.Dispatcher), + "Expected the global dispatcher to belong to Node's built-in undici." + + " Check that nothing loads npm undici before the first fetch call." + ); + }); + + // Whether cross-instance dispatch actually breaks depends on the exact + // pairing between Node's built-in undici and our npm undici copy, so this + // repro is skipped on Node versions where the two happen to be compatible. + it("repro: enqueueing through the foreign dispatcher fails with a transport error", async (t) => { + const client = new mindee.Client({ + apiKey: apiKey, + dispatcher: foreignDispatcher, + }); + const source = new mindee.BufferInput({ + buffer: pdfBuffer, + filename: "blank.pdf", + }); + try { + await client.enqueue( + mindee.product.Extraction, source, { modelId: modelId } + ); + } catch (err: unknown) { + assert.ok( + !(err instanceof mindeeHttp.MindeeHttpErrorV2), + "Expected a transport-level error, not an API error." + ); + return; + } + t.skip( + "Cross-instance dispatch is compatible on this Node version;" + + " the foreign-dispatcher breakage cannot be reproduced here." + ); + }); + + it("fix: the default client ignores the foreign dispatcher and succeeds", async () => { + const client = new mindee.Client({ apiKey: apiKey }); + const source = new mindee.BufferInput({ + buffer: pdfBuffer, + filename: "blank.pdf", + }); + const response = await client.enqueue( + mindee.product.Extraction, source, { modelId: modelId } + ); + assert.ok(response.job.id); + }); + + it("fix: UrlInput ignores the foreign dispatcher and downloads succeed", async () => { + const source = new mindee.UrlInput({ + url: "https://github.com/mindee/client-lib-test-data/blob/main/" + + "file_types/pdf/blank_1.pdf?raw=true", + }); + assert.notStrictEqual(source.dispatcher, foreignDispatcher); + assert.ok(source.dispatcher instanceof undici.Dispatcher); + await source.init(); + const localized = await source.asLocalInputSource(); + await localized.init(); + assert.ok(localized.fileObject.length > 0); + }); + } +);