Skip to content

fix: buffer write calls until ephemeral server address is resolved - #915

Open
saiyam-jain-dev wants to merge 1 commit into
forwardemail:masterfrom
saiyam-jain-dev:fix-streaming-write
Open

saiyam-jain-dev wants to merge 1 commit into
forwardemail:masterfrom
saiyam-jain-dev:fix-streaming-write

Conversation

@saiyam-jain-dev

Copy link
Copy Markdown

Fixes #911

Problem

In 7.3.1, ephemeral server binding resolves asynchronously (app.listen(0, '127.0.0.1')), so serverAddress() temporarily returns a placeholder URL without the ephemeral port. For standard requests, end() formats the real URL once listening before calling Superagent's super.end().

However, streaming requests using .write() invoke Superagent's this.request() eagerly, which initializes and caches this.req against the placeholder URL (default port 80), resulting in ECONNREFUSED.

Solution

  • Buffer .write() calls when the ephemeral server address is not yet resolved.
  • Once the server emits listening, or when end() dispatches, format this.url with the resolved ephemeral port and flush the buffered write chunks to the request stream.
  • Add regression test in test/issue-fixes.js.

@JH8459

JH8459 commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Thanks for picking this up. I wrote #907, so this regression came from my change.

I ran this branch (ca792f6) next to 7.3.0 and 7.3.1 on macOS with Node 24.11 and superagent 10.3.0. The repro from #911 fails on 7.3.1 and passes here. So do writes with a delay between them, an explicit encoding, 20 1MB chunks staying in order, Readable.pipe(test), https, and using await or expect instead of end(). A listen error after a buffered write still reaches the caller, the bind test from #907 still passes, and the full suite is 138 passing.

One difference I noticed: calling test.request() before end() worked in 7.3.0 but still goes to port 80 here, since it doesn't go through write(). I have no idea how many people do that and I don't think it needs to block this PR, it might just be worth a follow-up. test.pipe(writable) throws on 7.3.0 as well, so that one isn't new.

The CI run seems to be waiting on approval, so I thought these results might help in the meantime.

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.

7.3.1: streaming requests (.write) against an app factory hit port 80 (ECONNREFUSED)

2 participants