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

Make nodes commit errors when the retrieval script panics #2311

Open
tmpolaczyk opened this issue Nov 21, 2022 · 1 comment
Open

Make nodes commit errors when the retrieval script panics #2311

tmpolaczyk opened this issue Nov 21, 2022 · 1 comment

Comments

@tmpolaczyk
Copy link
Contributor

When tally execution panics, the data request is resolved as "RadError::Unknown". I would expect a similar behavior if the retrieval or aggregation scripts panic, but that's not the case. Instead of commiting a "RadError::Unknown", nodes do not commit anything. So most probably the data request will resolve with an "insufficient commits" error.

This is similar to #2306, but this behavior has always been like that, ever since the creation of the RadManager. This is the piece of code that decides to ignore this case:

.map_err(move |e| {
// If resolving a data request results in a panic in the
// message handler, ignore this data request
log::error!("Couldn't resolve rad request {}: {}", dr_pointer, e)
})

So not sure if this is intended or not. But we could even have nodes reporting the panic message, perhaps as a new RadError.

@aesedepece
Copy link
Member

The behavior is different for tallies vs. commits because data requests need to be eventually tallied, so there shouldn't be such a thing as refraining from tallying.

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

No branches or pull requests

2 participants