Repository navigation
Log a failing initial background image instead of rejecting unhandled - #148
Merged
Merged
Conversation
The constructor called the async update() without awaiting or catching it, so an imagePath that failed to load surfaced as an unhandled promise rejection that callers had no way to handle. It is now caught and logged, matching how init() handles the same failure. Adds e2e case D6, which constructs the processor with a 404ing image and asserts the page reports no uncaught error. Fixes #141
🦋 Changeset detectedLatest commit: e6005dd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
1egoman
approved these changes
Oct 21, 2026
Comment on lines
64
to
67
| const fileSet = await vision.FilesetResolver.forVisionTasks( | ||
| this.options.assetPaths?.tasksVisionFileSet ?? | ||
| `https://cdn.jsdelivr.net/npm/@mediapipe/tasks-vision@${dependencies['@mediapipe/tasks-vision']}/wasm`, | ||
| ); |
Contributor
There was a problem hiding this comment.
suggestion: would it also make sense to fetch fileSet in the constructor as well for a similar reason (frontloading a potentially large request?)
Contributor
Author
There was a problem hiding this comment.
Yes. But will do in a separate PR.
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
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 #141.
What
BackgroundTransformer's constructor calls the asyncupdate()without awaiting it. If the initialimagePathfailed to load (404, CORS, aborted request), the rejection had no handler. It reached the page as an unhandled promise rejection with no message (the value is a DOMEvent), and callers had no way to catch it.The constructor now attaches a
.catch()that logs the failure, matching howinit()already handles the same load.Why not move the load into
init()The issue suggests that as the preferred fix. I kept the constructor preload because apps often construct the processor well before
setProcessor()(on a pre-join screen, for example), and the preload letsinit()reuse the already-decoded image. Functionally nothing changes:init()still retries the load and logs.init()still skips the fetch because the image is already cached.Test
Adds e2e case D6: the harness constructs the processor with a background image that returns 404, then the test asserts no
pageerrorand no harness error. It also checks the image was actually requested, so it can't pass trivially. With the fix reverted, D6 fails with an uncaught page error"Event".Includes a patch changeset.