Repository navigation
Suppress more errors due to closed documents - #22321
nicolo-ribaudo wants to merge 2 commits into
Conversation
Viewer previewCommit a8fd84c (build logs).
Previews are removed when the PR closes and may be pruned after 30 days without an update. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #22321 +/- ##
==========================================
+ Coverage 90.52% 90.57% +0.04%
==========================================
Files 278 278
Lines 68586 68603 +17
==========================================
+ Hits 62090 62136 +46
+ Misses 6496 6467 -29
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
839d681 to
9145897
Compare
Merging this PR will not alter performance
Comparing Footnotes
|
timvandermeij
left a comment
There was a problem hiding this comment.
Looks good; thank you!
Closing/switching the document in the viewer will cause multiple tasks to fail. Other code paths already have `if (pdfDocument !== this.pdfDocument) return;` checks to know if the document has been changed after an async step, but it was missing in some places leading to errors being logged in the console. This commit adds the check in those places.
This is so that those promises don't remain pending forever, and their callers can clean up associated resources.
9145897 to
a8fd84c
Compare
|
@timvandermeij I updated the PR to reject the promise. I left it as two commits because one might argue that the second one is semver-major (it turns a forever-pending promise into a rejected one). I personally think it's more of a bugfix than a breaking change, but if you prefer I can move it to a separate PR that can wait until when we're ready for PDF.js 6. |
| try { | ||
| await pdfViewer.pagesPromise; | ||
| } catch { | ||
| break; // The document was closed, or failed to load. |
There was a problem hiding this comment.
Here we're in an async function, but the caller is ignoring its errors anyway so this try-catch prevents the "unhandled rejection" warning that browsers report.
I noticed some errors in my console while switching between PDFs. Unfortunately it's very racy so I'm not sure if I updated all the places, but these are the ones I noticed.
Commit messages: