Skip to content

Instruments that guard DOM access with typeof window cannot be uploaded, because the bundle shadows window with a throwing Proxy #1802

Open

Description

@joshunrau

Instrument code that guards its DOM access with typeof window !== 'undefined' still throws when the API evaluates the bundle, so the instrument cannot be uploaded or imported. Where the host has no DOM, createBundle's GLOBALS preamble shadows document, self and window with Proxy objects that throw on any property access. They exist to give a better error message, but a Proxy is an object, so typeof window is 'object' on the server. The standard guard then passes, and the guarded window.… access hits the throwing trap. The UMD-style typeof self !== 'undefined' ? self : this check found in many copied-in libraries fails the same way. InstrumentsService.create evaluates every bundle in a Node vm with no DOM, so such an instrument is rejected with Failed to interpret instrument bundle (422), or skipped during a repository import, even though it is correct browser code.

Where

packages/instrument-bundler/src/bundle.ts:19-35:

  const __createProxy = (name) => {
    // ...
    return new Proxy({ name }, {
      get(target, property) {
        throw new Error(formatErrorMessage('get', property.toString(), target.name))
      },
      // ...
  };
  const document = globalThis.document ?? __createProxy('document');
  const self = globalThis.self ?? __createProxy('self');
  const window = globalThis.window ?? __createProxy('window');

Reproduce

  1. Bundle a form whose index.ts has, at module scope, const lang = typeof window !== 'undefined' ? window.navigator.language : 'en'; and export default { kind: 'FORM', lang, … }.
  2. Evaluate the bundle the way the API does: vm.runInContext(bundle, vm.createContext({})), or upload it through POST /v1/instruments.

Actual: Error: Cannot get property 'navigator' of object 'window' in global scope of file 'index.ts' (verified on main with vm), and the upload fails with 422.
Expected: the guard sees no window on the server, the bundle evaluates, and the instrument is stored. Unguarded DOM access at module scope still fails with a message naming the file.

Tests

packages/instrument-bundler/src/__tests__/bundle.test.ts: it('should let a typeof window guard skip DOM access when evaluated without a DOM, so guarded instruments can be uploaded'). Evaluate the bundled string with vm.runInContext in an empty context.

e2e, in testing/src/specs/admin-instruments.spec.ts (or wherever the upload flow is covered): uploading an instrument with a guarded module-scope window access succeeds.

Suggested fix

Drop the shadowing proxies, so document, self and window are truly undeclared when the host has none. Keep the helpful message another way: wrap the evaluation in createBundle's IIFE in a try/catch that rethrows a ReferenceError for document, self or window with globalThis.__ODC_BUNDLER_ERROR_CONTEXT attached, so unguarded access still names the file.

Activity

  1. added
    BugType: existing behavior is wrong
    Priority: LowNice to have, cosmetic or speculative
    Difficulty: MediumStraightforward change, about a day or less
    Area: InstrumentsInstrument authoring API, bundling, runtime, rendering, measures and the built-in library
    on Oct 7, 2026
  2. joshunrau commented on Oct 26, 2026

    @joshunrau
    CollaboratorAuthor

    Still reproduces on main (d86740e). No commit since this was filed touches bundle.ts or plugin.ts, and no open PR touches bundle.ts.

    The same consts also break bundling

    A top-level binding named document, self or window in any input collides with the preamble's declarations (bundle.ts:33-35). bundle() then throws The symbol "document" has already been declared from the esbuild.transform in createBundle (bundle.ts:60). It is a plain esbuild error, not an InstrumentBundlerError, located at a line of the synthetic wrapper (<stdin>:30 here), so it names neither the author's file nor their line, and the playground shows no CodeErrorBlock. It fails wherever bundle() runs: the playground (esbuild-wasm 0.28.2 reports the same error), repository imports and the CLI.

    Reproduced with bundle({ inputs: [{ name: 'index.ts', content: "const document = 'x'; export default { kind: 'FORM', document };" }] }). A top-level function window() {}, or export const self in a second file, fails the same way. Bindings inside a function body are fine. Dropping the declarations fixes this too, and it needs its own test.

    On the suggested fix

    Dropping the proxies is right. Three details for the catch:

    • Test error instanceof ReferenceError rather than matching the identifier in the message, whose wording differs by engine (Safari says Can't find variable: x). The bundle throws and catches in its own realm, so instanceof holds in the vm context and in the browser. Catching every ReferenceError also names the file for navigator, localStorage and similar globals, which the proxies never covered: an unguarded navigator.language fails today with a bare navigator is not defined.
    • Keep the original message in the new error, e.g. new ReferenceError(error.message + " in global scope of file '" + context + "'", { cause: error }), and rethrow everything else unchanged. InstrumentInterpreter already wraps evaluation errors (packages/instrument-interpreter/src/index.ts:28), and InstrumentErrorFallback shows one level of cause (packages/react-core/src/components/InstrumentErrorFallback/InstrumentErrorFallback.tsx:33-35), so a generic wrapper message would push the real one out of view.
    • The try must enclose ${inject} and return __exports as well as ${output.js}. The createBundle tests pass const __exports (packages/instrument-bundler/src/__tests__/bundle.test.ts:8), which a return after the block could not see.

    Checked with esbuild 0.28.2, the same transform options and an empty vm context: this shape evaluates the guarded case, lets typeof self !== 'undefined' ? self : globalThis fall through, rejects window.innerWidth with window is not defined in global scope of file 'index.ts', and passes a TypeError through unchanged.

    packages/instrument-bundler/AGENTS.md:48-56 and .agents/docs/architecture/instrument-pipeline.md:83-86 describe the proxies and need updating in the same commit.

    Tests

    Unit, in bundle.test.ts: the issue's case, an unguarded access, a TypeError passing through, and bundle() accepting a top-level document. Assert on error.name, because the error comes from the vm realm and fails an outer instanceof ReferenceError. Write the sources without /runtime/v1 imports, unlike the repositories/ fixtures: the bundle turns those into a dynamic import(), which an empty vm context rejects with ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING.

    e2e: the suite cannot build a fresh bundle. Its only POST /v1/instruments re-posts a seeded one (testing/src/specs/authorization.spec.ts:402-405, through readSeededBundle at :278-284). A hand-written bundle string is enough for #1712 and #1706, but tests nothing here, because the preamble is what changes. Two options:

    1. testing/src/specs/playground.spec.ts: open a share link with the guarded source, log in with the API base URL pointed at the test API, and upload through Upload Bundle, expecting the 201 success notification. No new dependency, and one test covers the browser bundler and the API evaluation. Put uniqueId in internal.name, since a repeated name and edition is a 409. build: upgrade dependencies held back by breaking changes #1660, fix(playground): search instruments by label #1817 and fix: resolve 26 low-difficulty issues, one commit each #1825 also edit this spec.
    2. testing/src/specs/admin-instruments.spec.ts: call bundle() in the test and POST the result with adminToken. Shorter, and Instrument upload validation ignores the instrument's language, accepting untranslated bilingual instruments and rejecting language: ['en'] #1712 and FILE instrument schema accepts file groups that can never be completed: min above max, or two groups sharing a basename #1706 could build their bundles the same way. But it adds @opendatacapture/instrument-bundler, and esbuild with it, to testing/package.json, a new dependency that needs agreeing first.

    I would take option 1.

    Rollout: repository imports bundle inside the API (apps/api/src/instrument-repos/instrument-repos.service.ts:280), so an API redeploy covers them. Playground uploads need a playground redeploy, and CLI users a release after 2.5.2 and a rebuild. Stored bundles need nothing: each one has already evaluated on the server.

    Triage note written with Claude Code. Code references point at main @ d86740e.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Area: InstrumentsInstrument authoring API, bundling, runtime, rendering, measures and the built-in libraryBugType: existing behavior is wrongDifficulty: MediumStraightforward change, about a day or lessPriority: LowNice to have, cosmetic or speculative

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions