Ensure that the PDFDocumentLoadingTask is rejected when "setting up fake worker" failed (issue 10135)
#10139
Conversation
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/c365bd8f80112e1/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/c76daefe802cf66/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/c76daefe802cf66/output.txt Total script time: 3.99 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/c365bd8f80112e1/output.txt Total script time: 5.33 mins
|
Snuffleupagus
force-pushed
the
issue-10135
branch
2 times, most recently
from
334e701 to
1d143a7
Compare
From: Bot.io (Linux m4)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/dbd9861cbbe867d/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/c9c1407a76eeea8/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/dbd9861cbbe867d/output.txt Total script time: 3.95 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/c9c1407a76eeea8/output.txt Total script time: 5.17 mins
|
Snuffleupagus
force-pushed
the
issue-10135
branch
from
1d143a7 to
c62d46c
Compare
|
I think that this should now account for all the different cases. /botio unittest |
From: Bot.io (Linux m4)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/32ba4254f6fb5d5/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/3229c127ff8a6a3/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/32ba4254f6fb5d5/output.txt Total script time: 3.90 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/3229c127ff8a6a3/output.txt Total script time: 5.44 mins
|
|
/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/d7525a20583caaa/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/d7525a20583caaa/output.txt Total script time: 2.93 mins Published |
… fake worker" failed (issue 10135) This should, hopefully, cover all the possible ways[1] in which "fake workers" are loaded. Given the different code-paths, adding unit-tests might not be that simple. Note that in order to make this work, the various `fakeWorkerFilesLoader` functions were converted to return `Promises`. --- [1] Unfortunately there's lots of them, for various build targets and configurations.
Snuffleupagus
force-pushed
the
issue-10135
branch
from
c62d46c to
755c6ed
Compare
| if (typeof PDFJSDev !== 'undefined' && PDFJSDev.test('GENERIC')) { | ||
| let useRequireEnsure = false; |
There was a problem hiding this comment.
The latest update only moved this definition here, since it's unused in e.g. mozilla-central and this way it gets removed from the output.
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/cd033cefdf5bf81/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.215.176.217:8877/94daebadbb61b99/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/cd033cefdf5bf81/output.txt Total script time: 19.81 mins
|
From: Bot.io (Windows)FailedFull output at http://54.215.176.217:8877/94daebadbb61b99/output.txt Total script time: 24.04 mins
Image differences available at: http://54.215.176.217:8877/94daebadbb61b99/reftest-analyzer.html#web=eq.log |
|
Thank you for improving the error handling here! |
This should, hopefully, cover all the possible ways[1] in which "fake workers" are loaded. Given the different code-paths, adding unit-tests might not be that simple. Edit: Also given that "fake workers" are only intended as a fallback anyway, and aren't really tested currently, the patch is hopefully OK even without tests.
Note that in order to make this work, the various
fakeWorkerFilesLoaderfunctions were converted to returnPromises. Edit: Smaller diff with https://github.com/mozilla/pdf.js/pull/10139/files?w=1Fixes #10135.
[1] Unfortunately there's lots of them, for various build targets and configurations.