RDBC-1083: a few remarks addressed

Author: Scooletz

src/Http/RavenCommand.ts

@@ -162,64 +162,81 @@ export abstract class RavenCommand<TResult> {
 
         let optionsToUse: RequestInit | BunFetchRequestInit;
 
-        if (RuntimeUtil.isBun()) {
-            optionsToUse = { body: bodyToUse, ...restOptions } as BunFetchRequestInit;
-        } else if (RuntimeUtil.isWorkerd()) {
-            // Cloudflare Workers' fetch has no undici `dispatcher` concept; mTLS is
-            // handled by the custom fetch (an mtls_certificate binding). Passing a
-            // dispatcher here is meaningless and can confuse the runtime.
-            optionsToUse = { body: bodyToUse, ...restOptions } as RequestInit;
-        } else {
+        if (RuntimeUtil.supportsUndiciDispatcher()) {
             if (dispatcher) { // support for fiddler
                 agent = dispatcher;
             }
             optionsToUse = { body: bodyToUse, ...restOptions, dispatcher: agent } as RequestInit;
+        } else {
+            // Bun and Cloudflare Workers (workerd) have no undici `dispatcher` concept, so
+            // one must never leak into their fetch init. On workerd mTLS is handled by the
+            // custom fetch (an mtls_certificate binding); Bun manages TLS itself.
+            optionsToUse = { body: bodyToUse, ...restOptions } as RequestInit;
         }
 
-        let passthrough: PassThrough;
-        try {
-            passthrough = new PassThrough();
-        } catch (err) {
-            throw RavenCommand._maybeNodeStreamUnavailableError(err as Error);
-        }
-        passthrough.pause();
+        // `new PassThrough()`, `Readable.fromWeb`, `new Stream()` and `.pipe` are all
+        // `node:stream` APIs; an incomplete edge polyfill (Nitro/unenv) can throw from any
+        // of them. `_guardNodeStream` turns those into an actionable error. PassThrough is
+        // built BEFORE the fetch on purpose: on a broken polyfill we fail fast without
+        // issuing the (possibly state-mutating) request.
+        const passthrough = RavenCommand._guardNodeStream(() => {
+            const pt = new PassThrough();
+            pt.pause();
+            return pt;
+        });
 
         const fetchFn = fetcher ?? fetch; // support for custom fetcher
         const response = await fetchFn(uri, optionsToUse);
 
-        // `Readable.fromWeb` / `new Stream()` / `.pipe` are all `node:stream` APIs,
-        // so an incomplete edge polyfill (Nitro/unenv) can throw here just like
-        // `new PassThrough()` above. Guard them so the actionable error is raised.
-        try {
+        RavenCommand._guardNodeStream(() => {
             const effectiveStream: Stream =
                 response.body
                     ? Readable.fromWeb(response.body)
                     : new Stream();
 
             effectiveStream
                 .pipe(passthrough);
-        } catch (err) {
-            throw RavenCommand._maybeNodeStreamUnavailableError(err as Error);
-        }
+        });
 
         return {
             response,
             bodyStream: passthrough
         };
     }
 
+    // Runs a `node:stream` operation, translating an incomplete-polyfill failure into an
+    // actionable error. Shared by every node:stream call site in send() so none can be
+    // added without the guard, and the translation logic lives in one place.
+    private static _guardNodeStream<T>(fn: () => T): T {
+        try {
+            return fn();
+        } catch (err) {
+            throw RavenCommand._maybeNodeStreamUnavailableError(err as Error);
+        }
+    }
+
     private static _maybeNodeStreamUnavailableError(err: Error): Error {
-        // Some edge bundlers ship an incomplete `node:stream` where PassThrough throws
-        // "not implemented" (e.g. Nitro/unenv, used by TanStack Start & Nuxt). Turn that
-        // cryptic stub error into an actionable one pointing at the real fix.
-        if (RuntimeUtil.isWorkerd()) {
+        // The failure is signalled by the polyfill itself, not by the runtime: unenv
+        // (used by Nitro-based presets -- TanStack Start, Nuxt -- and edge targets such as
+        // Cloudflare Workers, Vercel Edge, Deno Deploy) ships `node:stream` stubs that
+        // throw e.g. "[unenv] PassThrough is not implemented yet!". We match that signature
+        // rather than the runtime so (a) every affected runtime gets the guidance, not just
+        // workerd, and (b) an unrelated node:stream error on a correctly configured runtime
+        // is NOT mislabeled as a polyfill problem. The original error is kept as the cause.
+        const message = err?.message ?? "";
+        const isIncompletePolyfill = message.includes("[unenv]")
+            || /\bnot implemented\b/i.test(message);
+
+        if (isIncompletePolyfill) {
             return getError(
                 "InvalidOperationException",
-                "The RavenDB client requires a working node:stream on Cloudflare Workers, but this "
-                + "runtime provided an incomplete polyfill. Enable Node.js compatibility by adding "
-                + `compatibility_flags = ["nodejs_compat"] (and a recent compatibility_date) to your `
-                + "wrangler configuration. For framework bundlers such as Nitro / TanStack Start / Nuxt "
-                + "this makes node: built-ins resolve to the workerd runtime instead of unenv stubs.",
+                "The RavenDB client requires a working node:stream, but this runtime provided an "
+                + "incomplete polyfill (unenv). On Cloudflare Workers, enable Node.js compatibility by "
+                + `adding compatibility_flags = ["nodejs_compat"] (and a recent compatibility_date) to `
+                + "your wrangler configuration; for framework bundlers such as Nitro / TanStack Start / "
+                + "Nuxt this makes node: built-ins resolve to the runtime instead of unenv stubs. On "
+                + "other edge runtimes (e.g. Vercel Edge, Deno Deploy) enable the equivalent Node.js "
+                + "compatibility so node:stream is backed by a real implementation.",
                 err);
         }
         return err;

src/Http/RequestExecutor.ts

@@ -340,17 +340,14 @@ export class RequestExecutor implements IDisposable {
             return null;
         }
 
-        if (RuntimeUtil.isBun()) {
-            return null;
-        }
-
-        if (RuntimeUtil.isWorkerd()) {
-            // Cloudflare Workers has no undici Agent / Node https agent.
-            // mTLS is provided via conventions.customFetch (an mtls_certificate binding).
-            // If a certificate was configured (the Node way) but no customFetch was set
-            // (checked above), the cert can never be presented -- fail fast with an
-            // actionable message instead of silently issuing an uncertified request.
-            if (this._certificate) {
+        // Runtimes whose fetch has no undici dispatcher (Bun, Cloudflare Workers) can't use
+        // an http agent -- there is nothing to build.
+        if (!RuntimeUtil.supportsUndiciDispatcher()) {
+            // On workerd a client certificate configured the Node way can never be presented
+            // (no Node https agent) unless mTLS is wired through conventions.customFetch
+            // (checked above). Fail fast with an actionable message instead of silently
+            // issuing an uncertified request.
+            if (RuntimeUtil.isWorkerd() && this._certificate) {
                 throwError("InvalidOperationException",
                     "A client certificate was configured via authOptions, but Cloudflare Workers has no Node "
                     + "https agent to present it. On workerd, X.509 mTLS must be provided through a Cloudflare "

src/Utility/RuntimeUtil.ts

@@ -10,6 +10,10 @@ export class RuntimeUtil {
      */
     public static isBun(): boolean {
         if (this._isBun === null) {
+            // `process` is absent on some edge runtimes (e.g. Cloudflare Workers/workerd),
+            // where a bare reference would throw a ReferenceError at module-load time and
+            // crash the very load path this detection protects. Keep the `typeof` guard --
+            // do not "simplify" it away. See isWorkerd().
             this._isBun = typeof process !== "undefined" && !!process.versions?.bun;
         }
         return this._isBun;
@@ -32,4 +36,18 @@ export class RuntimeUtil {
         }
         return this._isWorkerd;
     }
+
+    /**
+     * Whether the current runtime can accept an undici `Dispatcher` (Node's undici
+     * Agent) in a `fetch` init. Node can; Bun and Cloudflare Workers (workerd) cannot --
+     * their `fetch` has no dispatcher concept, so building or passing one is meaningless.
+     *
+     * Centralized here so the two consumers -- request send (RavenCommand.send) and http
+     * agent creation (RequestExecutor.getHttpAgent) -- stay in lockstep: if they drift
+     * (an agent is built but the dispatcher is dropped on send, or vice versa) the
+     * original workerd failure returns. A future dispatcher-less runtime is added once.
+     */
+    public static supportsUndiciDispatcher(): boolean {
+        return !this.isBun() && !this.isWorkerd();
+    }
 }