fix: treat loadMedia timeout of 0 as a zero-ms timeout instead of no timeout - #171
Open
razchiriac wants to merge 1 commit into
Open
fix: treat loadMedia timeout of 0 as a zero-ms timeout instead of no timeout#171razchiriac wants to merge 1 commit into
razchiriac wants to merge 1 commit into
Conversation
…timeout Previously `if (timeout)` was a truthy check, so passing `timeout: 0` disarmed the timer entirely and loadMedia waited forever for media that never fires load/error — the opposite of what a caller passing 0 wants. Now any non-nullish timeout arms the timer, so `timeout: 0` resolves on the next macrotask while `undefined` keeps the previous wait-forever behavior for direct callers (createContext still defaults to 30000).
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.
Fixes the
timeout: 0handling inloadMedia(#170).Problem
if (timeout)is a truthy check, sotimeout: 0disarms the safety timer entirely andloadMediawaits forever for media that never firesload/error— the opposite of what a caller passing0wants, and inconsistent with theOptions.timeoutdocs ("millisecond, default: 30000").Change
timeout != nullarms the timer for any numeric value including0(resolves on the next macrotask).undefinedkeeps the current wait-forever behavior for direct callers, andcreateContextstill defaults to30000, so nothing changes for the normaldomTo*paths.Test
Added a case to
test/nodejs.test.ts: a happy-dom image never firesload/error, so before this changeloadMedia(img, { timeout: 0 })hangs (test fails via a 1s guard race); after it, the promise resolves immediately.We've been running with this behavior while capturing full-page screenshots of a dashboard-style web app in production.