More robust getPage() error handling. #8746
Conversation
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/28fd883f5e8450d/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/28fd883f5e8450d/output.txt Total script time: 2.33 mins Published |
| @@ -251,6 +251,10 @@ class PDFFindController { | |||
| // Store the pageContent as a string. | |||
| this.pageContents[i] = strBuf.join(''); | |||
| extractTextCapability.resolve(i); | |||
| }, (reason) => { | |||
There was a problem hiding this comment.
In most places in the patch you're using }).catch((reason) => {, but not here[1].
Just to satisfy my own curiosity: Is there a particular reason for not using that format here as well?
[1] I'm aware that there's a difference in behaviour between the two formats, since }, (reason) => { will only catch errors from the original promise whereas }).catch((reason) => { will also catch errors from the function inside of the then() above.
There was a problem hiding this comment.
Not exact science. Mostly I'm doing it by observation of then-handler: 1) if there is a logic that will cause exception or return a promise -- .catch() will be used; 2) however if logic have some simple finalization code thus no reason for exception, I'm trying to fit into then-catch-handler, it will avoid to perform additional micro task spawning.
| @@ -387,6 +387,11 @@ class PDFViewer { | |||
| if (--getPagesLeft === 0) { | |||
| pagesCapability.resolve(); | |||
| } | |||
| }, (reason) => { | |||
There was a problem hiding this comment.
The question in https://github.com/mozilla/pdf.js/pull/8746/files#diff-485f8990604b45b959a20e90265d8044R254 applies here as well.
|
Can we add console.error logs to all the places where we consume the rejections/exceptions? Previously, you'd at least see an unhandled promise rejection error to know something went wrong. |
yurydelendik
force-pushed
the
page-errors
branch
from
5b3ba91 to
d0e9372
Compare
Added few console.error |
More robust getPage() error handling.
Attempt to address https://bugzilla.mozilla.org/show_bug.cgi?id=1368927