Skip to content

fix: check monitor threads are still working after ibc tests#334

Merged
jjyr merged 1 commit intomainfrom
check-monitor-threads-after-ibc-tests
Oct 7, 2023
Merged

fix: check monitor threads are still working after ibc tests#334
jjyr merged 1 commit intomainfrom
check-monitor-threads-after-ibc-tests

Conversation

@jjyr
Copy link
Copy Markdown
Collaborator

@jjyr jjyr commented Sep 26, 2023

Fix monitor silently crashed issue #330 (comment)

We noticed sometimes CI returns success but the monitor thread is crashed silently, which is unexpected.

This PR adds code to check monitor is working at the end of ibc tests, and returns an error if it isn't working.

We call subscribe function of ChainEndpoint to check the monitor thread, if the monitor is crashed, the subscribe will return a SendError error, we converted it to Error::Relayer.

@jjyr jjyr requested review from ashuralyk and blckngm September 26, 2023 07:42
Copy link
Copy Markdown
Contributor

@ashuralyk ashuralyk left a comment

Choose a reason for hiding this comment

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

for just solving the throw of failures in monitor, this PR is supposed to merge

@ashuralyk
Copy link
Copy Markdown
Contributor

ashuralyk commented Oct 7, 2023

@jjyr I noticed the test between Axon and Axon has failures but CI also passed, please see this:
https://github.com/synapseweb3/forcerelay/actions/runs/6309594521/job/17174053398?pr=334 (in Run IBC Tests)

image

@jjyr
Copy link
Copy Markdown
Collaborator Author

jjyr commented Oct 7, 2023

Details

The discussion is moved to #339

This PR should be merged and #339 is not introduced in this PR.

@jjyr jjyr added this pull request to the merge queue Oct 7, 2023
Merged via the queue into main with commit 2a9eed1 Oct 7, 2023
@jjyr jjyr deleted the check-monitor-threads-after-ibc-tests branch October 7, 2023 08:12
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.

4 participants