Maintenance/workers compat#15
Conversation
- New CloudflareWorkersHandler in the request layer mirroring the Lambda/Node/Browser pattern, with cookie/header/redirect helpers aligned with crowdhandler-cloudflare-integration. - RequestContext + init() accept cloudflareWorkersRequest, with a matching function overload for type inference. - BaseClient detects workerd via navigator.userAgent and routes HTTP through native fetch + AbortController, preserving the axios error shape so existing handlers keep working. Node/Lambda continue to use axios unchanged. - package.json gains an exports map with workerd/worker conditions pointing Workers bundlers at the ESM build. - README documents Workers usage end-to-end.
There was a problem hiding this comment.
Pull request overview
Adds Cloudflare Workers (workerd) runtime compatibility to the SDK, including request-context handling and HTTP transport changes, while also centralizing CrowdHandler query parameter keys and improving redirect URL construction.
Changes:
- Add Cloudflare Workers request context support via a new
CloudflareWorkersHandlerandinit()config option. - Route API HTTP calls through native
fetchwhen running in Workers, keeping axios-like error/response shapes. - Centralize
ch-*query-string keys (CH_PARAM_KEYS) and update redirect/query processing to better handle hash fragments.
Reviewed changes
Copilot reviewed 9 out of 29 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/request/requestContext.ts | Adds Workers request context switching to return a Workers-specific handler. |
| src/request/cloudflareWorkersHandler.ts | Introduces a Workers-native request/response handler (Request in, Response out). |
| src/init.ts | Adds cloudflareWorkersRequest to config and overloads; wires it into context creation and error message. |
| src/gatekeeper/gatekeeper.ts | Centralizes ch-param stripping via CH_PARAM_KEYS and ensures ch-params are placed before URL hash fragments. |
| src/common/types.ts | Adds shared CH_PARAM_KEYS constant for ch-param detection/stripping. |
| src/common/processURL.ts | Uses CH_PARAM_KEYS to strip ch-params from query strings. |
| src/client/base_client.ts | Adds Workers detection and a fetch-based HTTP path intended to avoid axios usage in Workers. |
| package.json | Bumps version and adds conditional exports entries (including workerd/worker). |
| dist/types/request/requestContext.d.ts | Updates type declarations for Workers request context. |
| dist/types/request/cloudflareWorkersHandler.d.ts | Adds type declarations for the new Workers handler. |
| dist/types/init.d.ts | Updates type declarations for cloudflareWorkersRequest config overload. |
| dist/types/common/types.d.ts | Exposes CH_PARAM_KEYS in generated type declarations. |
| dist/types/client/base_client.d.ts | Adds httpRequest to generated type declarations. |
| dist/request/requestContext.js | Compiled output reflecting Workers request context support. |
| dist/request/cloudflareWorkersHandler.js | Compiled output for the new Workers handler. |
| dist/init.js | Compiled output reflecting cloudflareWorkersRequest initialization support. |
| dist/gatekeeper/gatekeeper.js | Compiled output reflecting updated redirect URL building and param stripping. |
| dist/crowdhandler.umd.js | Bundled UMD output updated for v2.4.0 and Workers support logic. |
| dist/crowdhandler.esm.js | Bundled ESM output updated for v2.4.0 and Workers support logic. |
| dist/crowdhandler.cjs.js | Bundled CJS output updated for v2.4.0 and Workers support logic. |
| dist/common/types.js | Compiled output exporting CH_PARAM_KEYS. |
| dist/common/processURL.js | Compiled output using CH_PARAM_KEYS for stripping. |
| dist/client/base_client.js | Compiled output for fetch-based Workers HTTP routing. |
| README.md | Documents Workers usage and updates versioned CDN snippet and feature list. |
Comments suppressed due to low confidence (1)
src/client/base_client.ts:273
httpDELETE(path, body)ignores itsbodyargument and does not send it in the request. This is a functional regression for callers likeResource.delete()that pass a payload (and the previous axios.delete implementation typically sent it asdata). Passbodythrough tohttpRequest()and ensure the Workers fetch path also supports a DELETE request body whenbodyis provided.
async httpDELETE(path: string, body: object) {
try {
const response = await this.httpRequest("DELETE", this.apiUrl + path, {
headers: {
"x-api-key": this.key,
},
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "workerd": "./dist/crowdhandler.esm.js", | ||
| "worker": "./dist/crowdhandler.esm.js", | ||
| "browser": "./dist/crowdhandler.umd.js", | ||
| "import": "./dist/crowdhandler.esm.js", | ||
| "require": "./dist/crowdhandler.cjs.js", | ||
| "default": "./dist/crowdhandler.esm.js" |
There was a problem hiding this comment.
The new conditional exports for workerd/worker point to ./dist/crowdhandler.esm.js, but that bundle still has a top-level import axios from 'axios'. In Workers environments this often breaks bundling/runtime unless Node compatibility/polyfills are enabled, which contradicts the goal of “no Node polyfills required”. Consider providing a dedicated Workers ESM build that does not import axios (or lazy-loads axios only in non-Workers runtimes) and update these export conditions to reference that file.
| "workerd": "./dist/crowdhandler.esm.js", | |
| "worker": "./dist/crowdhandler.esm.js", | |
| "browser": "./dist/crowdhandler.umd.js", | |
| "import": "./dist/crowdhandler.esm.js", | |
| "require": "./dist/crowdhandler.cjs.js", | |
| "default": "./dist/crowdhandler.esm.js" | |
| "workerd": "./dist/crowdhandler.umd.js", | |
| "worker": "./dist/crowdhandler.umd.js", | |
| "browser": "./dist/crowdhandler.umd.js", | |
| "import": "./dist/crowdhandler.esm.js", | |
| "require": "./dist/crowdhandler.cjs.js", | |
| "default": "./dist/crowdhandler.umd.js" |
| public setCookie( | ||
| value: string, | ||
| cookieName: string = "crowdhandler", | ||
| domain?: string | ||
| ): string { | ||
| // Returns the Set-Cookie header value — caller appends it to their | ||
| // outgoing Response. Format mirrors the existing CF integration. | ||
| const parts = [`${cookieName}=${value}`, "path=/", "Secure"]; | ||
| if (domain) { | ||
| parts.push(`domain=${domain}`); | ||
| } | ||
| return parts.join("; "); | ||
| } |
There was a problem hiding this comment.
CloudflareWorkersHandler.setCookie() returns a header value string but does not actually apply it anywhere. Because Gatekeeper.setCookie() calls this.REQUEST.setCookie(...) for side effects and ignores the return value, gatekeeper.setCookie() will report success in a Workers context while not setting any cookie. Consider either making Workers setCookie() throw (to prevent silent no-ops) or updating the Gatekeeper/handler contract so Workers can surface the Set-Cookie value to the caller in a supported way.
| clearTimeout(timeoutId); | ||
| // Mirror axios's "no response received" error shape so errorHandler's | ||
| // `else if (error.request)` branch fires. | ||
| const wrapped: any = new Error(err?.message || "Network request failed"); |
There was a problem hiding this comment.
In the Workers fetch path, a timeout-triggered abort will throw (typically a DOMException with name "AbortError"), but the wrapped error you rethrow does not set an axios-like code (e.g. ECONNABORTED). As a result, createError.apiConnection() will classify SDK timeouts as generic connection failures instead of API_TIMEOUT. Consider detecting aborts (via err.name === 'AbortError' / controller.signal.aborted) and setting wrapped.code accordingly to preserve existing timeout semantics.
| const wrapped: any = new Error(err?.message || "Network request failed"); | |
| const wrapped: any = new Error(err?.message || "Network request failed"); | |
| if (controller.signal.aborted || err?.name === "AbortError") { | |
| wrapped.code = "ECONNABORTED"; | |
| } |
- await responses().put() on Workers so it flushes inside ctx.waitUntil (the fire-and-forget put was cancelled when the isolate terminated) - send sampleRate in the put body to bypass server-side gating - expose optional timeout for recordPerformance (default 1500ms) - thread timeout through Resource.put → httpPUT → httpRequest - extract isCloudflareWorkers detection to common/runtime.ts - README: document timeout, fix stale factor example, add Workers note Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…rt, setCookie returns header string Agent-Logs-Url: https://github.com/Crowdhandler/crowdhandler-javascript-sdk/sessions/c6b7fcba-80a4-4ac9-af1a-0db689f102cd Co-authored-by: luke-owen-crowdhandler <60788788+luke-owen-crowdhandler@users.noreply.github.com>
|
Addressed all three review comments in commit d856796:
|
The UMD bundle references browser globals (e.g. `window` in js-sha256) that throw `ReferenceError: window is not defined` in the workerd runtime. ESM was the working path before d856796; restoring it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Picks up review-comment changes from d856796 (ECONNABORTED on abort, setCookie returns header string in Workers) which only updated src. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
No description provided.