refac(net, exchange, dht) network service errors #101
Conversation
Rather than pushing errors back down to lower layers, propagate the
errors upward.
This commit adds a `ReceiveError` method to BitSwap's network receiver.
Still TODO: rm the error return value from:
net.service.handler.HandleMessage
This is inspired by delegation patterns in found in the wild.
btc
force-pushed
the
feat/network-error-propagation-1
branch
from
d4036d0
to
623cb6c
Compare
|
Let me know if anything is needed before the merge. |
|
Rise! Rise! RISE!!! (errors, that is) |
|
Now we're cooking with gas. Merged locally |
|
by "merged locally" do you mean you merged it to master on your computer, then pushed? |
| @@ -20,7 +20,7 @@ type Handler interface { | |||
|
|
|||
| // HandleMessage receives an incoming message, and potentially returns | |||
| // a response message to send back. | |||
| HandleMessage(context.Context, msg.NetMessage) (msg.NetMessage, error) | |||
| HandleMessage(context.Context, msg.NetMessage) msg.NetMessage | |||
There was a problem hiding this comment.
curious: why couldnt the same error propagation work, and then have Service be the only one to send it over to through a channel?
There was a problem hiding this comment.
e.g.
func (w *whatever) HandleMessage(context.Context, msg.NetMessage) (msg.NetMessage, error) {
// do whatever
return nil, errors.New("beep boop")
}
// in service.go
func (s *Service) handleIncomingMessage(ctx context.Context, m msg.NetMessage) {
...
r1, err := s.Handler.HandleMessage(ctx, m2)
if err != nil {
go something.ReceiveError(err)
return
}
...
}There was a problem hiding this comment.
These changes are based on this discussion:
It's that it's weird to send errors back down to the service. What does it mean to return error in a handler function? What is the service meant to do with this error?
It would appear that the better thing to do is for the handler to log and perform error handling internally.
|
I'm late to the party but am worried that this -- which removes Maybe not, maybe it's apparent because you have to spin out goroutines which have to report errors the same way... idk. makes me uneasy cause now i can't look at a function signature and know clearly exactly how it fails. |
|
Recall #69 (comment) |
|
idk what's right. :/ yeah i recall now-- thanks for link. Looking at this code makes me uneasy though. So the tension is between:
and
both are important to know, but what's more important to interactions with the codebase going fwd? |
|
When receiving a raw ethernet packet, what does it mean to return an error to the device driver? Here, what does it mean to return an error to the network layer? |
|
Having error handling performed by the network layer does reduce code duplication though. Error handling is performed just once. |
Add an option to pass URL to zap
Let errors rise up up and away.
@jbenet @whyrusleeping