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.

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

[MM-54589] Report job start failure #46

Merged
merged 2 commits into from
Oct 4, 2023
Merged

[MM-54589] Report job start failure #46

merged 2 commits into from
Oct 4, 2023

Conversation

streamer45
Copy link
Contributor

@streamer45 streamer45 commented Oct 2, 2023

Summary

PR implements some logic to report a failure on start to the plugin side. This is especially needed to cover the case in which the bot fails to connect which would otherwise trigger a rather long timeout (currently set to 2 minutes). With this, if the browser process were to fail for any reasons, we'd be surfacing the error to the user right away.

As mentioned in #44 (comment) I'd probably this approach in place of the custom websocket event.

Related PR

mattermost/mattermost-plugin-calls#542

Ticket Link

https://mattermost.atlassian.net/browse/MM-54589

@streamer45 streamer45 added the 2: Dev Review Requires review by a core committer label Oct 2, 2023
@streamer45 streamer45 requested a review from cpoile October 2, 2023 22:28
@streamer45 streamer45 self-assigned this Oct 2, 2023
Copy link
Member

@cpoile cpoile left a comment

Choose a reason for hiding this comment

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

Nice!

@streamer45 streamer45 added 3: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Oct 3, 2023
@streamer45 streamer45 added this to the v0.4.3 milestone Oct 3, 2023
@streamer45 streamer45 added the Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) label Oct 3, 2023
@streamer45 streamer45 modified the milestones: v0.4.3, v0.5.0 Oct 4, 2023
* Refactor connectivity check to be more resilient

* Bump packages deps

* Report job start

* [MM-54747] Implement structured logging (#48)
@streamer45 streamer45 merged commit 77ceb17 into master Oct 4, 2023
@streamer45 streamer45 deleted the MM-54589 branch October 4, 2023 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3: Reviews Complete All reviewers have approved the pull request Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants