Skip to content

Commit ca2fc10

Browse files
committed
RDBC-1083: a few remarks addressed
1 parent 28fc7e7 commit ca2fc10

4 files changed

Lines changed: 46 additions & 11 deletions

File tree

scripts/addWorkerCondition.mjs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,17 @@ for (const [subpath, entry] of Object.entries(pkg.exports)) {
5757
patched++;
5858
}
5959

60+
// Fail the build loudly if nothing was patched. tshy rewrites the whole "exports"
61+
// map on every build, so if its output shape ever changes (e.g. the "import"
62+
// condition is renamed or nested differently) this script would silently produce a
63+
// package.json without the workerd/worker conditions and ship a broken package.
64+
if (patched === 0) {
65+
throw new Error(
66+
"addWorkerCondition: patched 0 export entries -- no entry exposed an 'import' condition to mirror. "
67+
+ "tshy's 'exports' shape likely changed; the workerd/worker conditions were NOT injected. "
68+
+ "Refusing to leave package.json without them.");
69+
}
70+
6071
writeFileSync(PKG_PATH, JSON.stringify(pkg, null, 2) + "\n");
6172
// Log to stderr so it never pollutes stdout of `npm pack --silent` (which is
6273
// parsed to obtain the tarball filename in CI).

src/Http/RavenCommand.ts

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,10 @@ export abstract class RavenCommand<TResult> {
151151
public async send(agent: Dispatcher,
152152
requestOptions: HttpRequestParameters): Promise<{ response: HttpResponse, bodyStream: Readable }> {
153153

154-
const { body, uri, fetcher, ...restOptions } = requestOptions;
154+
// `dispatcher` is pulled out here so it never leaks into `restOptions`: the
155+
// normal path re-adds it explicitly (see below), while the Bun and workerd
156+
// paths must not carry an undici dispatcher into their fetch init.
157+
const { body, uri, fetcher, dispatcher, ...restOptions } = requestOptions;
155158

156159
log.info(`Send command ${this.constructor.name} to ${uri}${body ? " with body " + body : ""}.`);
157160

@@ -167,8 +170,8 @@ export abstract class RavenCommand<TResult> {
167170
// dispatcher here is meaningless and can confuse the runtime.
168171
optionsToUse = { body: bodyToUse, ...restOptions } as RequestInit;
169172
} else {
170-
if (requestOptions.dispatcher) { // support for fiddler
171-
agent = requestOptions.dispatcher;
173+
if (dispatcher) { // support for fiddler
174+
agent = dispatcher;
172175
}
173176
optionsToUse = { body: bodyToUse, ...restOptions, dispatcher: agent } as RequestInit;
174177
}
@@ -184,13 +187,20 @@ export abstract class RavenCommand<TResult> {
184187
const fetchFn = fetcher ?? fetch; // support for custom fetcher
185188
const response = await fetchFn(uri, optionsToUse);
186189

187-
const effectiveStream: Stream =
188-
response.body
189-
? Readable.fromWeb(response.body)
190-
: new Stream();
190+
// `Readable.fromWeb` / `new Stream()` / `.pipe` are all `node:stream` APIs,
191+
// so an incomplete edge polyfill (Nitro/unenv) can throw here just like
192+
// `new PassThrough()` above. Guard them so the actionable error is raised.
193+
try {
194+
const effectiveStream: Stream =
195+
response.body
196+
? Readable.fromWeb(response.body)
197+
: new Stream();
191198

192-
effectiveStream
193-
.pipe(passthrough);
199+
effectiveStream
200+
.pipe(passthrough);
201+
} catch (err) {
202+
throw RavenCommand._maybeNodeStreamUnavailableError(err as Error);
203+
}
194204

195205
return {
196206
response,

src/Http/RequestExecutor.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,16 @@ export class RequestExecutor implements IDisposable {
347347
if (RuntimeUtil.isWorkerd()) {
348348
// Cloudflare Workers has no undici Agent / Node https agent.
349349
// mTLS is provided via conventions.customFetch (an mtls_certificate binding).
350+
// If a certificate was configured (the Node way) but no customFetch was set
351+
// (checked above), the cert can never be presented -- fail fast with an
352+
// actionable message instead of silently issuing an uncertified request.
353+
if (this._certificate) {
354+
throwError("InvalidOperationException",
355+
"A client certificate was configured via authOptions, but Cloudflare Workers has no Node "
356+
+ "https agent to present it. On workerd, X.509 mTLS must be provided through a Cloudflare "
357+
+ "mtls_certificate binding wired into conventions.customFetch (see the \"Cloudflare Workers\" "
358+
+ "section of the README). Remove the certificate from authOptions and set conventions.customFetch.");
359+
}
350360
return null;
351361
}
352362

src/Utility/RuntimeUtil.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,17 @@ export class RuntimeUtil {
1818
/**
1919
* Detects if the code is running in the Cloudflare Workers (workerd) runtime.
2020
*
21-
* workerd exposes the Web `navigator.userAgent` as the fixed string
21+
* workerd exposes the Web `navigator.userAgent` as the string
2222
* "Cloudflare-Workers", which is the officially documented way to detect it.
23+
* We match by prefix so a future version suffix (e.g. "Cloudflare-Workers/1")
24+
* still resolves to workerd instead of failing closed into the Node path
25+
* (which would then crash trying to build an undici agent that workerd lacks).
2326
*/
2427
public static isWorkerd(): boolean {
2528
if (this._isWorkerd === null) {
2629
const nav = (globalThis as unknown as { navigator?: { userAgent?: string } }).navigator;
27-
this._isWorkerd = !!nav && nav.userAgent === "Cloudflare-Workers";
30+
this._isWorkerd = !!nav && typeof nav.userAgent === "string"
31+
&& nav.userAgent.startsWith("Cloudflare-Workers");
2832
}
2933
return this._isWorkerd;
3034
}

0 commit comments

Comments
 (0)