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

Interface to register listeners for network errors on receiving side #129

Closed
dirkmc opened this issue Dec 10, 2020 · 2 comments · Fixed by #136
Closed

Interface to register listeners for network errors on receiving side #129

dirkmc opened this issue Dec 10, 2020 · 2 comments · Fixed by #136
Labels
need/triage Needs initial labeling and prioritization topic/filecoin Topic filecoin

Comments

@dirkmc
Copy link
Collaborator

dirkmc commented Dec 10, 2020

Currently there is a RegisterNetworkErrorListener to listen for errors that occur when sending a request.
It would be useful to also be able to listen for errors that occur while processing a stream on the receiving side. Suggested name: RegisterReceiverNetworkErrorListener

Motivation

When a client wants to send data to a provider

  1. go-data-transfer client opens a "push" channel to the provider
  2. go-data-transfer provider opens a go-graphsync query to the client for the file
  3. go-graphsync responds with the file data

If there is a network error (eg if the provider is restarted), the provider should wait for the client to restart the "push" channel.
The client can begin attempting to restart the channel as soon as graphsync reports a network error.

Needed for Miner should not try to dial the client to restart a data transfer

Implementation proposal

Currently when there is a network error on the receiving side, the networking layer calls ReceiveError(err). ReceiveError could use a pubsub mechanism to call the listeners, or the networking layer could do that directly.

Ideally the listener would take two parameters: func(peer.ID, error)

@dirkmc dirkmc added need/triage Needs initial labeling and prioritization topic/filecoin Topic filecoin labels Dec 10, 2020
@hannahhoward
Copy link
Collaborator

@dirkmc we can certainly propogate the errors in ReceiveError -- I don't know what it will actually communicate. I believe ReceiverError is only called when we attempt to decode a protobuf message from the stream and error. Seems to come up mostly around connection resets? Hard to know.

Anyway, I can definitely address this -- it's not a hard ticket, but I'm not sure whether it will solve whatever problem you're attempting to solve.

@dirkmc
Copy link
Collaborator Author

dirkmc commented Dec 11, 2020

@hannahhoward in my testing, when I restart the Provider, on the client side I see the log message
Graphsync ReceiveError: io: read/write on closed pipe
This is coming from ReceiveError.

It would be useful for the client to be informed of this error so that it can attempt to reconnect to the Provider.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/triage Needs initial labeling and prioritization topic/filecoin Topic filecoin
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants