Skip to content

Comments

Catch promise rejections from pushWithReply#3766

Merged
SteffenDE merged 2 commits intomainfrom
sd-pushWithReply-catch-v1
Apr 16, 2025
Merged

Catch promise rejections from pushWithReply#3766
SteffenDE merged 2 commits intomainfrom
sd-pushWithReply-catch-v1

Conversation

@SteffenDE
Copy link
Collaborator

#3443 changed the pushWithReply function to use promises instead. Previously, if we did not handle the timeout, it was just ignored, but now we reject the promise, which will cause unhandled promise rejections.

This PR adds catch clauses to all cases where we previously did not handle the timeout. At the moment, these log an error, but we can also silently drop if we want. cc @chrismccord

Supersedes #3760.

#3443
changed the pushWithReply function to use promises instead. Previously,
if we did not handle the timeout, it was just ignored, but now we reject
the promise, which will cause unhandled promise rejections.

This PR adds catch clauses to all cases where we previously did not handle
the timeout. At the moment, these log an error, but we can also silently
drop if we want. cc @chrismccord

Supersedes #3760.
@SteffenDE SteffenDE merged commit e617453 into main Apr 16, 2025
14 checks passed
@SteffenDE SteffenDE deleted the sd-pushWithReply-catch-v1 branch April 16, 2025 16:51
SteffenDE added a commit that referenced this pull request Apr 16, 2025
* Catch promise rejections from pushWithReply

#3443
changed the pushWithReply function to use promises instead. Previously,
if we did not handle the timeout, it was just ignored, but now we reject
the promise, which will cause unhandled promise rejections.

This PR adds catch clauses to all cases where we previously did not handle
the timeout. At the moment, these log an error, but we can also silently
drop if we want. cc @chrismccord

Supersedes #3760.

* don't catch for pushHookEvent
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant