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

Do not retry actions from CLI upon an Assertion Failed #2007

Closed
spalladino opened this issue Sep 5, 2023 · 7 comments
Closed

Do not retry actions from CLI upon an Assertion Failed #2007

spalladino opened this issue Sep 5, 2023 · 7 comments
Assignees
Labels
T-bug Type: Bug. Something is broken.

Comments

@spalladino
Copy link
Collaborator

If a user tries sending a tx that they know will fail, the retrying-fetch we have will still retry it 3 times (and log it those 3 times). We should detect an Assertion Failed error as a server error, and thus unretriable, and not keep retrying.

Requested on Discord:

QoL request re: makeFetch errors
when testing, I send a tx that I know will fail. it will print the error foundation:retry ERROR Error: Assertion failed: '...' three times via the makeFetch retry. This doesn't have any positive purpose IMO since the ts interface will throw an error for the dev to handle. It should probably also short-circuit the retry loop if the error is Assertion failed:
the fetch failed error SocketError: other side closed is common and clutters the cli too. Instead of logging this error, it should just throw if all retries fail and let the dev handle it
I know the api optionally takes DebugLogger but I am not providing it. I think the ideal situation would be to log retry errors only if the debug logger is provided.

@github-project-automation github-project-automation bot moved this to Todo in A3 Sep 5, 2023
@spalladino spalladino added the T-bug Type: Bug. Something is broken. label Sep 5, 2023
@spalladino spalladino added this to the 📢 Initial Public Sandbox Release milestone Sep 5, 2023
@benesjan benesjan self-assigned this Sep 7, 2023
@benesjan benesjan moved this from Todo to In Progress in A3 Sep 7, 2023
@benesjan
Copy link
Contributor

benesjan commented Sep 7, 2023

I just tried reproducing it by putting an assertion into private token's transfer function and when I run the test I get correct behavior:
In the sandbox log I get one log: aztec:simulator:acvm Error in oracle callback callPrivateFunction +25ms
And in the failed test output I get a nicely looking assertion error:
Image

@spalladino do you by any chance have any more info regarding how to reproduce this?

@spalladino
Copy link
Collaborator Author

Not really, all I got is that quote from Discord that Mike had shared.

@spalladino
Copy link
Collaborator Author

Can you try reproducing it from the CLI instead of from a test?

@benesjan
Copy link
Contributor

benesjan commented Sep 7, 2023

Same result using the CLI.

Log output in Sandbox:

Image

And the cute assertion message in CLI output:

Image

Makes me think whether it was not some old version of sandbox which didn't yet use the NoRetry errors.

@spalladino
Copy link
Collaborator Author

Maybe follow up with the guy who reported this on Discord, to see if they can still reproduce it with the latest version, or they can share the logs?

@benesjan benesjan moved this from In Progress to Blocked in A3 Sep 8, 2023
@benesjan
Copy link
Contributor

benesjan commented Sep 8, 2023

Marked this as blocked until I get a response from the user on Discord. Here is my message inquiring about more details.

It's also quite likely that this has already been addressed by our work on not retrying unrecoverable errors.

@spalladino
Copy link
Collaborator Author

Closing as stale

@spalladino spalladino closed this as not planned Won't fix, can't repro, duplicate, stale Oct 10, 2023
@github-project-automation github-project-automation bot moved this from Blocked to Done in A3 Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: Bug. Something is broken.
Projects
Archived in project
Development

No branches or pull requests

2 participants