-
Notifications
You must be signed in to change notification settings - Fork 48
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
fix: survive bad network requests #222
Conversation
When tasks are added to an existing list of tasks for a given peer, we need to sort the queue to ensure the order is correct, otherwise we never process pending tasks as task lists with pending tasks need to be moved up the queue. Fixes the build problems exposed in ipfs/js-ipfs#2992 Also upgrade aegir to a safe version.
Builds on the work in #221 I see intermittant but frequent CI errors with js-ipfs, usually after a test has finished and the ndoes are being torn down. Getting it to do some additional logging reveals bitswap is crashing when it cannot send a message to a remote peer due to the libp2p dial failing: ```console ipfs: [stdout] Error: stream ended before 1 bytes became available ipfs: at /home/travis/build/ipfs/js-ipfs/node_modules/it-reader/index.js:37:9 ipfs: at processTicksAndRejections (internal/process/task_queues.js:97:5) ipfs: at async /home/travis/build/ipfs/js-ipfs/node_modules/it-length-prefixed/src/decode.js:80:20 ipfs: at async oneChunk (/home/travis/build/ipfs/js-ipfs/node_modules/multistream-select/src/multistream.js:12:20) ipfs: at async Object.exports.read (/home/travis/build/ipfs/js-ipfs/node_modules/multistream-select/src/multistream.js:34:15) ipfs: at async module.exports (/home/travis/build/ipfs/js-ipfs/node_modules/multistream-select/src/select.js:21:19) ipfs: at async ClassIsWrapper.newStream [as _newStream] (/home/travis/build/ipfs/js-ipfs/node_modules/libp2p/src/upgrader.js:251:40) ipfs: at async ClassIsWrapper.newStream (/home/travis/build/ipfs/js-ipfs/node_modules/libp2p-interfaces/src/connection/connection.js:172:34) ipfs: at async Network.sendMessage (/home/travis/build/ipfs/js-ipfs/node_modules/ipfs-bitswap/src/network.js:147:34) ipfs: at async DecisionEngine._processTasks (/home/travis/build/ipfs/js-ipfs/node_modules/ipfs-bitswap/src/decision-engine/index.js:124:5) { ipfs: code: 'ERR_UNSUPPORTED_PROTOCOL', ipfs: buffer: BufferList { _bufs: [], length: 0 } ipfs: } ``` This PR adds a try/catch around network send operations. At the moment it just dumps the request, I'm not sure if we want to add a retry in there or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there is a network error, then the client should reconnect, and resend all its wants anyway, so I think it's ok to just log the error as you're doing here 👍
Could you rebase onto master now that I've merged #221
Done! Well, merged I've master into this PR.. |
Builds on the work in #221
I see intermittant but frequent CI errors with js-ipfs, usually after a test has finished and the nodes are being torn down. Getting it to do some additional logging reveals bitswap is crashing when it cannot send a message to a remote peer due to the libp2p dial failing:
This PR adds a try/catch around network send operations. At the moment it just dumps the request, I'm not sure if we want to add a retry in there or something.