fix(workspace): guard downloadAndSave against non-browser environments - #247
fix(workspace): guard downloadAndSave against non-browser environments#247VascoSch92 wants to merge 1 commit into
Conversation
RemoteWorkspace.downloadAndSave() uses browser-only DOM APIs (document.createElement, document.body, URL.createObjectURL). In a Node.js environment these throw a cryptic "ReferenceError: document is not defined". Add a runtime guard that throws a clear, actionable error pointing Node.js callers to downloadAsBlob() / downloadAsText() instead. Browser behavior is unchanged. Closes #114
Endpoint audit❌ 7 off-contract call(s) — not on the agent-server · classifiers: cloud
❌ Not on agent-server (gated, 7)⛔ (no known backend) — served by no backend we can see (6)
|
|
🤖 OpenHands is reviewing this PR. Head commit: This comment was posted by an AI agent (OpenHands). |
all-hands-bot
left a comment
There was a problem hiding this comment.
This review was created by an AI agent (OpenHands) on behalf of the repository maintainers.
Review Summary
This PR adds a runtime environment guard (typeof document === 'undefined') at the top of RemoteWorkspace.downloadAndSave(), throwing a clear, actionable error that directs Node.js callers to the existing Node-safe alternatives (downloadAsBlob() / downloadAsText()).
Findings
No material issues found. The change is clean, minimal, and well-targeted:
- Guard placement is correct — it runs before
downloadAsBlob()(the network call), so Node.js callers fail fast without touching the network, and the first test case explicitly verifies this. - Error message is actionable — both suggested fallbacks (
downloadAsBlob,downloadAsText) exist on the same class (lines 263/281) and are Node-safe. - Guard is sufficient — the method also uses
URL.createObjectURL/revokeObjectURL, but the guard throws before reaching those, so no partial-execution concern. - Tests are thorough — three cases covering the Node.js guard error (no network), browser-like DOM stub behavior (full anchor lifecycle + revoke), and custom filename honoring. Global stubs are cleaned up via try/finally.
- Browser-compatible — the guard itself uses only
typeof(no Node.js APIs), consistent with the repo's browser-compatibility constraint.
Risk Assessment
Low risk. Browser behavior is unchanged; Node.js callers receive a clear error instead of a cryptic ReferenceError: document is not defined. Non-breaking.
Problem
RemoteWorkspace.downloadAndSave()uses browser-only DOM APIs (document.createElement,document.body,URL.createObjectURL) with no environment check. In Node.js — a common environment forRemoteWorkspaceautomation — it throws a crypticReferenceError: document is not defined.Closes #114.
Change
Add a runtime guard at the top of
downloadAndSave():ReferenceError. The suggested fallbacks (downloadAsBlob(),downloadAsText()) already exist and are Node-safe.Tests
New
src/__tests__/remote-workspace-download.test.ts:testEnvironment: 'node') suite and confirms the network is never touched;document/URL, the anchor download still fires (createElement('a'),href,click,appendChild/removeChild,revokeObjectURL);saveAsFileName.Full unit suite green (274 tests),
tscbuild clean, eslint + prettier clean.