Fix handling of fetch errors #13937
Fix handling of fetch errors #13937
Conversation
There was a problem hiding this comment.
Looks good to me aside from this comment, but I'd like @Snuffleupagus to also have a look here since I'm not overly familiar with this code.
Snuffleupagus
changed the title
Testing: - delete the pdf file while the initial request is inflight - delete the pdf file after the initial request has finished Repeat for a small file and large file, exercising both one-off and chunked transports.
das7pad
force-pushed
the
jpa-fix-error-handling
branch
from
6687f2c to
291ffd3
Compare
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/b23abab2edae538/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/9828a06074c6f96/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/9828a06074c6f96/output.txt Total script time: 33.72 mins
Image differences available at: http://54.67.70.0:8877/9828a06074c6f96/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://3.101.106.178:8877/b23abab2edae538/output.txt Total script time: 40.22 mins
Image differences available at: http://3.101.106.178:8877/b23abab2edae538/reftest-analyzer.html#web=eq.log |
|
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/a792de60ea3b953/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/8f66734e01a01a6/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/a792de60ea3b953/output.txt Total script time: 4.60 mins
|
From: Bot.io (Windows)SuccessFull output at http://3.101.106.178:8877/8f66734e01a01a6/output.txt Total script time: 6.17 mins
|
|
Thank you for the review feedback @Snuffleupagus ! 🎉 |
Hi!
We at @overleaf noticed that the error handling of network downloads is incomplete in a few scenarios.
We are hosting a lot of user content on a large cluster of VMs that scales with load. As VMs go, users will see 404s for downloads that reference PDFs on an old VM. Our user base is very diverse and so are the networks they access this content from, leading to occasional increases of network errors that are out of our control.
When downloads fail, our frontend got stuck in waiting for PDF.js to render or cleanup an instance -- despite already seeing network errors from the OS/browser in the Developer Tools.
These errors would turn up in the console as unhandled promise rejections and never reached our frontend code.
This PR is making sure that network errors make it to the call-sites.
Testing:
Repeat for a small file and large file, exercising both one-off and chunked transports.
We are seeing good results with the patch (rebased on top of v2.2.288) in production over the course of the past 2 months.
I ran the test suite as per the docs and it passed -- apart from a few pixel imperfections that seem to be unrelated to the patch, see example below.