fix: proxy ESM loading, progress bar suppression, and temp directory handling#375
Merged
MarshallOfSound merged 9 commits intomainfrom Mar 25, 2026
Merged
Conversation
Adds test/bugs.spec.ts with 6 failing tests that demonstrate 5 bugs: 1. GotDownloader progress bar suppression uses || instead of &&, so neither quiet:true nor ELECTRON_GET_NO_PROGRESS alone suppresses the progress bar (src/GotDownloader.ts:52). 2. GotDownloader leaks its 30s progress timer when a download fails because clearTimeout is not in a finally block (src/GotDownloader.ts:79-91). 3. initializeProxy() calls require() which is undefined in ESM, silently breaking proxy support (src/proxy.ts:36). 4. validateArtifact() does not forward tempDirectory to the nested SHASUMS256.txt download, so checksums are written to os.tmpdir() regardless of user config (src/index.ts:77-88). 5. validateArtifact() leaks an empty temp directory when cacheMode is ReadOnly/Bypass and checksums are not provided, because the cleanup finally deletes the wrong directory (src/index.ts:111-120).
The package is published as ESM ("type": "module"), so bare
require() is undefined at runtime. The try/catch swallowed the
ReferenceError, causing proxy support to silently never work.
Define require via createRequire(import.meta.url) so the optional
dependency can be loaded synchronously as before.
The condition used || instead of &&, so neither `quiet: true` alone nor ELECTRON_GET_NO_PROGRESS alone suppressed the progress bar. Both had to be set simultaneously, contradicting the documented behavior.
If pipeline() threw, the 30-second progress-bar timer was never cleared and would fire after the download had already failed, rendering a progress bar for a dead download. Move the cleanup into a finally block.
The nested downloadArtifact call for checksum validation forwarded most options but omitted tempDirectory, so SHASUMS256.txt was always written under os.tmpdir() regardless of the caller's configuration. This breaks setups where os.tmpdir() is read-only or on a different filesystem.
validateArtifact created its temp directory with ORPHAN mode when the caller owned the output (ReadOnly/Bypass cache modes), but the validation tempFolder never needs to outlive the function. When checksums were not provided, the inner finally block deleted the SHASUMS256 download directory instead, leaving an empty electron-download-* directory behind on every ReadOnly/Bypass download. Always use CLEAN mode for the validation tempFolder.
url.parse() and url.format() (legacy) are deprecated since Node.js 11. Switch to the WHATWG URL API. The new implementation produces identical cache-directory hashes for all real-world Electron download URLs (verified against github.com, nightlies, ports, and userinfo). Only the synthetic test fixture URL with no path component hashes differently, so the hardcoded expected hash in Cache.spec.ts is updated.
Split the tests from bugs.spec.ts into: - test/GotDownloader.spec.ts (progress bar and timer cleanup) - test/proxy.spec.ts (createRequire guard) - test/index.spec.ts (tempDirectory handling, added to existing downloadArtifact describe block)
Lock in the cache-directory hash for a real Electron release URL so any future URL-parsing change that would invalidate on-disk caches fails this test instead of silently shipping.
erickzhao
approved these changes
Mar 25, 2026
|
🎉 This PR is included in version 4.0.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes five bugs uncovered during a code audit, plus one deprecation cleanup. Each fix is a separate commit.
Bugs fixed
initializeProxy()silently broken in ESMsrc/proxy.tscalled barerequire('global-agent'), but the package is published as"type": "module"whererequireis undefined. The try/catch swallowed theReferenceError, so proxy support never worked and users got no indication why. Fixed by definingrequireviacreateRequire(import.meta.url).Progress bar suppression condition inverted
src/GotDownloader.ts:52used!quiet || !process.env.ELECTRON_GET_NO_PROGRESS, which requires both flags to suppress the bar. Changed to&&so either flag works as documented.Progress timer leaks on download failure
When
pipeline()threw,clearTimeoutwas never called, leaving a 30-second timer armed that would render a progress bar for an already-failed download. Moved cleanup into afinallyblock.SHASUMS256.txt download ignores custom
tempDirectoryThe nested
downloadArtifactcall for checksum validation forwarded most options but omittedtempDirectory, so checksums were always written underos.tmpdir()regardless of caller configuration.validateArtifactleaks empty temp directoriesWhen
cacheModewasReadOnlyorBypassand checksums were not provided, the validation temp folder was created withORPHANmode but never cleaned up (the innerfinallydeleted the wrong directory). The validation folder never needs to outlive the function, so it now always usesCLEANmode.Refactor
Replace deprecated
url.parse()with WHATWG URLCache.getCacheDirectory()used the legacyurl.parse/url.formatAPI. Switched tonew URL(). Verified identical cache-directory hashes for all real-world Electron download URLs (github.com releases, nightlies, ports, userinfo); only the syntheticdummy://test fixture hash changed.Tests
New regression tests live in:
test/GotDownloader.spec.ts— progress bar suppression + timer cleanup (non-network)test/proxy.spec.ts—createRequireguardtest/index.spec.ts—tempDirectorypropagation + leak check (added to existingdownloadArtifactsuite)73 tests passing, lint clean, build clean.