Desktop: Fixes #10740: Improve the reliability of fetching resources #10813
Conversation
This file contains 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
pedr
changed the title
laurent22
reviewed
Aug 2, 2024
packages/lib/shim-init-node.ts
Outdated
| @@ -630,6 +630,7 @@ function shimInit(options: ShimInitOptions = null) { | |||
| }); | |||
|
|
|||
| request.on('timeout', () => { | |||
| if (!requestOptions.timeout) return; | |||
There was a problem hiding this comment.
Could you add a comment here please? I know we cannot test this, but some information on why we do that could help future developers.
|
Could it make sense to target |
Auto-updated using release-website.sh
…th other parts of the app (laurent22#10636)
pedr
force-pushed
the
remove-timeout-from-fetchBlob
branch
from
c7ab7c7
to
13c2463
Compare
|
Closing in favour of #10826 since it should be targeting 3.0 |
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: #10740
Summary
As reported by the user, the new code, when
timeoutis set to undefined, uses thehttp.globalAgentwhich has a default timeout to 5 seconds.Previously, this wouldn't be a problem because we didn't have an event handler for
timeoutevent, so now I'm checking if the timeout was specified for this request, and if it wasn't I opt to not destroy the request.I also added more information to the log message when the request timeout.
Testing
To test this change I created a simple HTTP server, with a sleep function in the endpoint that returns one of the images from a static HTML page.
After that, I used the Web Clipper to save the full HTML page, which will use the
fetchBlobfunction to download the image.The sleep function should be higher than the timeout value, if that happens we can see the
timeoutevent being generated.