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

fix: gracefully handle remote stream closure during ping #2822

Merged
merged 2 commits into from
Nov 15, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions packages/protocol-ping/src/ping.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { randomBytes } from '@libp2p/crypto'
import { ProtocolError, TimeoutError } from '@libp2p/interface'
import { ProtocolError, TimeoutError, setMaxListeners } from '@libp2p/interface'
import { byteStream } from 'it-byte-stream'
import { equals as uint8ArrayEquals } from 'uint8arrays/equals'
import { PROTOCOL_PREFIX, PROTOCOL_NAME, PING_LENGTH, PROTOCOL_VERSION, TIMEOUT, MAX_INBOUND_STREAMS, MAX_OUTBOUND_STREAMS } from './constants.js'
Expand Down Expand Up @@ -60,10 +60,12 @@
const { stream } = data
const start = Date.now()
const bytes = byteStream(stream)
let pinged = false

Promise.resolve().then(async () => {
while (true) {
const signal = AbortSignal.timeout(this.timeout)
setMaxListeners(Infinity, signal)
signal.addEventListener('abort', () => {
stream?.abort(new TimeoutError('ping timeout'))
})
Expand All @@ -74,15 +76,34 @@
await bytes.write(buf, {
signal
})

pinged = true
}
})
.catch(err => {
this.log.error('incoming ping from %p failed with error', data.connection.remotePeer, err)
// ignore the error if we've processed at least one ping, the remote
// closed the stream and we handled or are handling the close cleanly
if (pinged && err.name === 'UnexpectedEOFError' && stream.readStatus !== 'ready') {
return
}

Check warning on line 88 in packages/protocol-ping/src/ping.ts

View check run for this annotation

Codecov / codecov/patch

packages/protocol-ping/src/ping.ts#L87-L88

Added lines #L87 - L88 were not covered by tests

this.log.error('incoming ping from %p failed with error - %e', data.connection.remotePeer, err)
stream?.abort(err)
})
.finally(() => {
const ms = Date.now() - start
this.log('incoming ping from %p complete in %dms', data.connection.remotePeer, ms)

const signal = AbortSignal.timeout(this.timeout)
setMaxListeners(Infinity, signal)

stream.close({
signal
})
.catch(err => {
this.log.error('error closing ping stream from %p - %e', data.connection.remotePeer, err)
stream?.abort(err)

Check warning on line 105 in packages/protocol-ping/src/ping.ts

View check run for this annotation

Codecov / codecov/patch

packages/protocol-ping/src/ping.ts#L104-L105

Added lines #L104 - L105 were not covered by tests
})
})
}

Expand Down
Loading