Skip to content

Conversation

beyondkmp
Copy link
Contributor

@beyondkmp beyondkmp commented Apr 30, 2025

fix #9063

Copy link

changeset-bot bot commented Apr 30, 2025

🦋 Changeset detected

Latest commit: 0a16de6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
electron-updater Patch

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.on("end", () => {
setTimeout(() => {
timeoutId = setTimeout(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleted.

Copy link
Collaborator

@mmaietta mmaietta May 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@beyondkmp can you please follow up here when you have a chance? Then we can proceed with the PR

@beyondkmp beyondkmp requested a review from mmaietta May 1, 2025 09:09
@github-actions github-actions bot added the Stale label Oct 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Differential download must fail after 10 seconds

2 participants