Skip to content

Commit

Permalink
fix: don't use the request signal in the agent (#324)
Browse files Browse the repository at this point in the history
Following an upgrade from `13.0.1` to `14.0.1` (which included upgrading
`@npmcli/agent` from `2.0.0` to `3.0.0`), we've seen strange behaviour:
a request works the first time, then immediately times out after that.

We're setting a `signal` on each request, which due to changes in
`@npmcli/agent` is then also used by the agent for connections. The
problem is that the original signal is used for all requests due to
agent caching. Once it's been used (i.e. after the first time a request
is made), no connections are possible (as they're immediately rejected).
This change resolves our issue by removing the signal from the agent.

I looked into adding a test, but I don't think it's possible without
making real network requests.

---------

Co-authored-by: Gar <wraithgar@github.com>
  • Loading branch information
thewilkybarkid and wraithgar authored Oct 16, 2024
1 parent 2248215 commit fa5f233
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion lib/remote.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ const RETRY_TYPES = [
// following redirects (through the cache if necessary)
// and verifying response integrity
const remoteFetch = (request, options) => {
const agent = getAgent(request.url, options)
// options.signal is intended for the fetch itself, not the agent. Attaching it to the agent will re-use that signal across multiple requests, which prevents any connections beyond the first one.
const agent = getAgent(request.url, { ...options, signal: undefined })
if (!request.headers.has('connection')) {
request.headers.set('connection', agent ? 'keep-alive' : 'close')
}
Expand Down

0 comments on commit fa5f233

Please sign in to comment.