fix(utils): classify browser Web Workers as browser runtime in HttpClient - #401
Conversation
…ient determineRuntime() equated 'no window' with server-side, so the signals Web Worker fell into 'server-with-fetch' and the ConvertAgent User-Agent announcement (meant for server-side SDK traffic only) ran inside real browsers. The resulting custom header forced a CORS preflight the signals endpoint rejects, breaking every signals worker upload and driving the WAF retry storm. Browser worker scopes are now detected via importScripts, which server runtimes (Node.js) and edge runtimes (Cloudflare Workers) do not expose, so those keep announcing ConvertAgent as intended. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Empty release commit so @convertcom/js-sdk ships a new version carrying the patched @convertcom/js-sdk-utils dependency. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request updates the runtime detection logic in packages/utils/src/http-client.ts to correctly identify browser Web Workers by checking for the presence of self and self.importScripts, preventing them from being misclassified as server runtimes. The feedback suggests using optional chaining when accessing importScripts on self to prevent potential TypeErrors if self is null in mocked or custom environments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
typeof null is 'object', so a mocked environment defining self = null passed the typeof guard and the importScripts access would throw. Optional chaining degrades it to undefined and falls through to the server checks, matching the file's existing navigator?.sendBeacon idiom. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Associated Ticket
https://app.asana.com/1/145550540855/project/1204450923340067/task/1215427653611922
What does this PR do?
Description:
determineRuntime()inpackages/utils/src/http-client.tsequated "nowindow" with server-side, so the signals Web Worker classified asserver-with-fetchand the ConvertAgentUser-Agentannouncement (server-side only by design) ran inside real browsers. The resulting custom request header forces a CORS preflight the signals endpoint rejects → every signals worker upload fails → the worker retry loop turns each sampled tab into ~2 preflights/second (~190M extra requests/day on the signals web ACL, worker-batched Signals collection dead since Jun 3rd).importScripts, which only exists in browser workers — Node.js and edge runtimes (Cloudflare Workers) don't expose it, so server-side and edge traffic keeps announcingConvertAgent/1.0for the metrics-endpointisConvertAgentUAbot-filter bypass exactly as intended.fix(js-sdk):commit so@convertcom/js-sdkalso receives a release carrying the patched utils dependency.Verified:
HttpClient.requestagainst a local capture server announcesConvertAgent/1.0in a plain Node runtime and stops injecting the header when a worker scope (self.importScripts) is present.@convertcom/js-sdk-utilsbuilds clean and its suite passes 120/120.Related: convertcom/backend#6694 (tracking build: stop property-mangling the header name + CORS-faithful signals tests), convertcom/backend#6695 (signals endpoint: authorize the headers, stops the WAF bleed for already-deployed clients).
What type of PR is this? (check all applicable)
Added tests?
(
determineRuntimeis module-private; the integration-level regression test lives in convertcom/backend#6694 — the signals browser suite now exercises real cross-origin CORS and asserts upload delivery, which fails if this class of bug ships again.)QA Instructions, Screenshots, Recordings
yarn workspace @convertcom/js-sdk-utils build && yarn workspace @convertcom/js-sdk-utils test:mochaHttpClientfrom the built lib in Node against a local HTTP server → request carriesUser-Agent: ConvertAgent/1.0; setglobalThis.self = {importScripts(){}}and repeat → header no longer injectedcd public/js/tracking && yarn build:testing && yarn test:signals --reporter=liststays 9/9