Replace panic with error in snap sync

[teor2345]

Panicking in async code causes a race condition which hangs the node roughly half the time on my machine. (The other half it exits with an error.)

As part of this race, the Ctrl-C handler is dropped, because it is an async future running in the same executor. So the user can't use Ctrl-C to exit.

The only way I found to exit the process was kill -KILL. A SIGTERM was ignored.

Close #3175.

Code contributor checklist:

• I have read, understood and followed contributing guide

[nazar-pc]

Thanks!

[teor2345]

This is the other place where we panic in async code. Is it a logic error, or should we replace it with returning an error to the (async) caller?

[nazar-pc]

This is a hard one. On one hand this is not a local decision that can be made in this context, on the other hand if we get to this point, it is a very fundamental application composition that I would say justified crashing the whole app since a lot of things will not work properly if this is actually the case.

Ultimately though we should probably simply remove gossipsub until we actually use it (it can be enabled or disabled during instantiation of the networking and it is currently disabled in 100% of cases and was for a very long time).

[shamil-gadelshin]

This change contradicts the previous PR: #3044
Should we invoke process exiting instead?

[nazar-pc]

It doesn't really. It is still an error, previous version just assumed that no segments means they don't exist, while in practice it might be caused by network issues.

[shamil-gadelshin]

We don't treat the new error as critical on the upper level, do we?

[nazar-pc]

It'll still get stuck I think, so shouldn't make a lot of difference in practice.

[shamil-gadelshin]

Not sure I follow. What I mean by the initial remark is that we have rolled back the PR which improved UX and if we want to remove the panic then we should propagate and/or handle the error correctly.

[nazar-pc]

It ended up being worse UX due to hanging. I think what we should do is to not eat an error in snap_sync and return it up the stack instead, so the whole DSN sync task can exit, which is an essential task and will bring down the whole node with it.

[shamil-gadelshin]

I mean the same when I suggest error propagation.