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

[RPC] Improve client error recovery #1270

Merged
merged 2 commits into from
Aug 25, 2021
Merged

[RPC] Improve client error recovery #1270

merged 2 commits into from
Aug 25, 2021

Conversation

m-yac
Copy link
Contributor

@m-yac m-yac commented Aug 25, 2021

Incorporates a change from argo which should resolve #1259

This PR also adds a test which shows that the client can now continue after an error, but does not include the exact example given in the issue above. Is it worth trying to get working an example exactly like the above? (i.e. using safe with different SAT solvers?)

@m-yac m-yac requested review from atomb and pnwamk August 25, 2021 17:59
@pnwamk
Copy link
Contributor

pnwamk commented Aug 25, 2021

LGTM modulo that minor comment on the test. It's not a big deal, but it would be nice to perhaps replace the reset with a call or eval invocation and a unit test assertion on the result ¯\_(ツ)_/¯

I approved the changes as I don't have anything else to add other than the above comment

@m-yac
Copy link
Contributor Author

m-yac commented Aug 25, 2021

With the exception of an expected "Unknown state ID" error (this time on ubuntu!) the relevant bits of the CI all pass, so I'm merging this.

@m-yac m-yac merged commit 4f16b02 into master Aug 25, 2021
@m-yac m-yac deleted the rpc/client-error-recovery branch August 25, 2021 19:07
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.

RPC Client exception handling broken
2 participants