feat(ipld): wrap bindnode with panic protection #368
Conversation
There was a problem hiding this comment.
I don't have a problem with this change, though I was thinking that graphsync would add a recover on every goroutine that handles a user's request asynchronously. That's akin to what net/http does: it spawns a goroutine to handle each HTTP request, and if any of it panics, then it doesn't bring the entire server down.
Recovering around bindnode is still better than no recover at all, but it doesn't cover us for the other thirty moving pieces: codecs, ipld-prime, car, etc.
|
mm, but I'm not sure that approach is going to work, or at least it might be difficult to pull off, since graphsync maintains pools of workers for some tasks and shared workers for others; it's not as straightforward as one overarching goroutine per request. |
|
That's fair, I just wanted to point that out. Being safe from bindnode panics is nice, but it's not the only worrying part of graphsync :) |
I meant to do this earlier but got stuck with other things. Just simple panic protection around bindnode use since bindnode is so new and we're still ironing out issues with it.