fix(differentialDownloader): improve error handling and timeout management in multipleRangeDownloader #9071
Conversation
…ement in multipleRangeDownloader
🦋 Changeset detectedLatest commit: 0a16de6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| response.pipe(dicer) | ||
|
|
||
| response.on("end", () => { | ||
| setTimeout(() => { | ||
| timeoutId = setTimeout(() => { |
There was a problem hiding this comment.
Do we need to store timeoutId for anything? Curious as to why we need to have wrappedReject. I was under the impression setTimeout without storing its ID would auto-release when GC'd
There was a problem hiding this comment.
The reject does seem unnecessary now, as it has already failed. At the time of writing, it was done for consistency, so both resolve and reject were included.
There was a problem hiding this comment.
I still see timeoutId but only being cleared on resolve now. Why can't we reuse previous logic and only change the timeout?
Are we simply detecting whether the timeout is working properly? e.g. are we attempting to resolve and reject the promise in the same flow so we're hitting a race condition?
There was a problem hiding this comment.
@beyondkmp can you please follow up here when you have a chance? Then we can proceed with the PR
fix #9063