Page MenuHomePhabricator

fast-ec panics should be passed up as regular Err's and rendered
Open, Needs TriagePublicBUG REPORT

Event Timeline

Hmm so I'm looking at the fast-ec repo now and can't quite tell what this task is concerned with? The GET /count endpoint already looks to match on Ok vs. Err types of the statistics variable.

All that said, I did happen to look into the fast_ec::go method and saw pool.disconnect().await.unwrap() which I guess could be returned to the caller as well... but I wasn't quite sure if that was the only location that was intended to be fixed (if at all).

Generally the codebase has quite a few unwraps, excepts, indexing operations, etc that all panic a fair amount. Currently these all panic and crash the Tokio thread, so the idea with this task is to make them all bubble up the error and show an error page to the user, instead of panicking. Perhaps this could be done using one of the "standard" Rust crates for the purpose. Those include anyhow, eyre, etc etc.

Ah the note about the unwrap() calls definitely helps :) Okay lemme take another whirl at the issue and see if I can't see how to play around with some of the usual error crates!

So here is what I have so far: https://gist.github.com/chusteven/a3a88711f859dc29fde7d7b697123de0

What was throwing me off so far was the run_query method, particularly in trying to "return early" (so to speak) from inside the conn.query_stream.await?.for_each() invocation... I tried looking at how this was done here on stackoverflow but didn't make it too far.

I'll keep poking at it, but also wanted to share an early diff in case I needed to be directed a different direction 😅