Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Handle admin command errors #725

Merged
merged 1 commit into from
Nov 30, 2022
Merged

Handle admin command errors #725

merged 1 commit into from
Nov 30, 2022

Conversation

AndrewFerr
Copy link
Member

Fixes #724

The crash caused by this kind of admin command errors is a regression in 2.0.2-rc1, but in 2.0.1, they would be ignored & the bridge bot would just say "Done".

@AndrewFerr AndrewFerr added S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems. labels Nov 29, 2022
@AndrewFerr AndrewFerr requested a review from a team as a code owner November 29, 2022 23:24
@AndrewFerr AndrewFerr self-assigned this Nov 29, 2022
@AndrewFerr
Copy link
Member Author

Summary: I mistakenly believed that Promise.finally caught rejected Promises. This PR adds a reject callback to admin command handlers so that asynchronously-raised errors in commands are properly caught & logged.

Reminder that the reason why the callback's handler function doesn't simply return a Promise is because yargs doesn't like it when parse is called multiple times with a command that returns a Promise (see the 4th Note here).

tadzik
tadzik reviewed Nov 30, 2022
View reviewed changes
src/Main.ts Show resolved Hide resolved
src/AdminCommand.ts Show resolved Hide resolved
@AndrewFerr AndrewFerr merged commit e883f81 into develop Nov 30, 2022
@AndrewFerr AndrewFerr deleted the af/handle-admin-command-errors branch November 30, 2022 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tadzik tadzik tadzik approved these changes

Assignees

@AndrewFerr AndrewFerr

Labels
S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

join room crashes the bridge
2 participants
@AndrewFerr @tadzik