Skip to content
This repository has been archived by the owner on Aug 23, 2019. It is now read-only.

fix: drop connection when stream ends unexpectedly #262

Merged
merged 2 commits into from
May 31, 2018

Commits on May 30, 2018

  1. fix: drop connection when stream ends unexpectedly

    Pull streams pass true in the error position when the sream ends.
    In https://github.com/multiformats/js-multistream-select/blob/5b19358b91850b528b3f93babd60d63ddcf56a99/src/select.js#L18-L21
    ...we're getting lots of instances of pull-length-prefixed stream
    erroring early with `true` and it's passed back up to the dialer
    in https://github.com/libp2p/js-libp2p-switch/blob/fef2d11850379a4720bb9c736236a81a067dc901/src/dial.js#L238-L241
    
    The `_createMuxedConnection` contains an assumption that any error
    that occurs when trying `_attemptMuxerUpgrade` is ok, and keeps the
    relveant baseConnecton in the cache. If the pull-stream has ended
    unexpectedly then keeping the connection arround starts causing
    the "already piped" errors when we try and use the it later.
    
    This PR adds a guard to avoid putting the connection back into the
    cache if the stream has ended.
    
    There is related work in an old PR to add a check for exactly this issue in
    pull-length-prefixed dignifiedquire/pull-length-prefixed#8
    ...but it's still open, so this PR adds a check for true in
    the error position at the site where the "already piped" errors
    were appearing. Once the PR on pull-length-prefixed is merged this
    check can be removed. It's not ideal to have it in this code as it
    is far removed from the source, but it fixes the issue for now.
    
    Arguably anywhere that `msDialer.handle` is called should do the
    same check, but we're not seeing this error occur anywhere else so
    to keep this PR small, I've left it as the minimal changeset to
    fix the issue.
    
    Of note, we had to add '/dns4/ws-star.discovery.libp2p.io/tcp/443/wss/p2p-websocket-star'
    to the swarm config to trigger the "already piped" errors. There
    is a minimal test app here https://github.com/tableflip/js-ipfs-already-piped-error
    
    Manual testing shows ~50 streams fail in the first 2 mins of
    running a node, and then things stabalise with ~90 active muxed
    connections after that.
    
    Fixes #235
    Fixes ipfs/js-ipfs#1366
    See dignifiedquire/pull-length-prefixed#8
    
    License: MIT
    Signed-off-by: Oli Evans <oli@tableflip.io>
    olizilla committed May 30, 2018
    Configuration menu
    Copy the full SHA
    0cef5cc View commit details
    Browse the repository at this point in the history

Commits on May 31, 2018

  1. Configuration menu
    Copy the full SHA
    d7428e5 View commit details
    Browse the repository at this point in the history