Skip to content

fix: handle terminal middleware without mutating chains#3534

Open
dozyio wants to merge 5 commits into
libp2p:mainfrom
dozyio:fix/dup-middleware
Open

fix: handle terminal middleware without mutating chains#3534
dozyio wants to merge 5 commits into
libp2p:mainfrom
dozyio:fix/dup-middleware

Conversation

@dozyio

@dozyio dozyio commented Jun 6, 2026

Copy link
Copy Markdown
Collaborator

Description

Fixes two stream middleware issues:

  • Copies registered inbound middleware before appending the protocol handler wrapper, preventing handlers from accumulating across streams.
  • Allows middleware to return false to stop the chain without aborting the stream.

Adds regression coverage for repeated inbound streams, terminal sync/async middleware, outbound terminal middleware, and throw/reject abort behaviour.

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

@dozyio dozyio requested a review from a team as a code owner June 6, 2026 10:44
@dozyio dozyio marked this pull request as draft June 6, 2026 10:58
@dozyio dozyio marked this pull request as ready for review June 6, 2026 18:24
@dozyio dozyio marked this pull request as draft June 6, 2026 18:24
@dozyio dozyio marked this pull request as ready for review June 7, 2026 08:24
*/
export interface StreamMiddleware {
(stream: Stream, connection: Connection, next: (stream: Stream, connection: Connection) => void): void | Promise<void>
(stream: Stream, connection: Connection, next: (stream: Stream, connection: Connection) => void): void | false | Promise<void | false>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be void | boolean | Promise<void | boolean>?

Comment on lines +371 to +379
if (result === false) {
stopChain()
} else if (result != null) {
result.then(result => {
if (result === false) {
stopChain()
}
}).catch(reject)
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just await mw call?

/**
* Stream middleware allows accessing stream data outside of the stream handler
*
* Return false to stop the middleware chain without aborting the stream.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could said returning true or void lets the middleware continue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants