fix(chromium): Fixed silent reload failures on Chrome 125 and earlier - #3836
Open
MaxFreedomPollard wants to merge 1 commit into
Open
MaxFreedomPollard wants to merge 1 commit into
MaxFreedomPollard wants to merge 1 commit into
Conversation
Reloading an extension on Chrome 125 and earlier printed "Last extension reload" even when the reload had not happened, and a failure inside the reload ended the web-ext process on an unhandled rejection instead of logging the usual reload error. reloadAllExtensions called reloadAllExtensionsFallbackForChrome125andEarlier without awaiting it, so the returned promise was dropped. The method wrote the success line and resolved with a plain result while the reload was still pending, which left any rejection with nowhere to go and kept it away from MultiExtensionRunner.handleReloadResults. Adding the await lets it propagate into the existing reloadError plumbing.
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
1 participant
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.
web-ext run -t chromium can report a reload that never happened and then exit. Pressing R or saving a file prints "Last extension reload: " straight away, and if the reload actually failed the process dies on an unhandled rejection instead of logging the usual error. This only affects the --load-extension fallback path, which runs when the browser has no Extensions.loadUnpacked CDP command, so Chrome 125 and earlier or a Chromium build without it.
The cause is a dropped promise. reloadAllExtensions called this.reloadAllExtensionsFallbackForChrome125andEarlier() with no await (src/extension-runners/chromium.js:398). That method is async and its first statement awaits Target.getTargets (chromium.js:445), so the call returned a pending promise, the success line was written, and the function resolved with a plain result. The error therefore never reaches MultiExtensionRunner.handleReloadResults, which only reports a failure when a result carries reloadError (src/extension-runners/index.js:73-88 and 170-181). Nothing in src/ or bin/ installs an unhandledRejection handler, so Node ends the whole process instead.
The change is the await. Nothing else in src/ needed touching. What it surfaces are the transport-level failures in the fallback: Target.getTargets, Target.createTarget, Target.attachToTarget, the explicit "Unexpectedly, no sessionId from attachToTarget" throw, and Runtime.evaluate, plus a pipe that drops mid-reload, where the CDP client rejects every pending response. Per-extension reload failures inside the code evaluated on chrome://extensions/ are deliberately left as they are: they already go through log.error and log.warn further down the same method, and that stays soft.
Letting the rejection propagate is the choice here because MultiExtensionRunner.reloadAllExtensions replaces a runner's resolved value with { runnerName } (src/extension-runners/index.js:78-81), so rejecting is the only way a reload error can reach handleReloadResults and be logged as 'Error occurred while reloading on "Chromium" - '. This does leave the two branches of reloadAllExtensions asymmetric: the Chrome 126 and newer branch catches a failed Extensions.loadUnpacked per extension and logs it (chromium.js:405-407) rather than rejecting. If you would rather have them match, try { await ... } catch (e) { log.error(e); } also fixes the crash and gets the failure logged, but the success line still prints afterwards, so that half of the bug would stay.
One test was added to tests/unit/test-extension-runners/test.chromium.js, next to the existing "falls back to --load-extension when needed (old Chrome)" test, which already builds a runner with loadUnpackedUnsupported: true. It runs the runner so it reaches the fallback the way it does in the field, stubs cdp.sendCommand to reject, then asserts both halves of the bug: that reloadAllExtensions() rejects, and that nothing containing "Last extension reload" was written to stdout. Without the await the test sees a fulfilled result instead of a rejection, which is the bug.
eslint and prettier --check pass on the two changed files.