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

ProxyAgent loop on SocketError #3897

Open
DTrombett opened this issue Nov 28, 2024 · 3 comments
Open

ProxyAgent loop on SocketError #3897

DTrombett opened this issue Nov 28, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@DTrombett
Copy link
Contributor

DTrombett commented Nov 28, 2024

Bug Description

When the connection to the proxy server is closed with UND_ERR_SOCKET using ProxyAgent an infinite loop is created.
This is actually a double bug; I found this while investigating why ProxyAgent doesn't work with a basic http proxy built with node:http:

import { ok } from "node:assert";
import { createServer, request as httpRequest } from "node:http";
import { request as httpsRequest } from "node:https";
import { Client, ProxyAgent, request } from "undici";

const server = createServer((req, res) => {
    ok(req.url);
    const targetUrl = new URL(req.url);
    req.pipe(
        (targetUrl.protocol === "https:" ? httpsRequest : httpRequest)(
            {
                method: req.method,
                hostname: targetUrl.hostname,
                port: targetUrl.port,
                path: targetUrl.pathname + targetUrl.search,
                headers: req.headers,
            },
            (proxiedRes) => {
                ok(proxiedRes.statusCode);
                res.writeHead(
                    proxiedRes.statusCode,
                    proxiedRes.statusMessage,
                    proxiedRes.headers,
                );
                proxiedRes.pipe(res);
            },
        ).once("error", res.destroy.bind(res)),
    );
}).listen();
const address = server.address();
ok(address && typeof address === "object");

// This works fine:
console.log(
    await new Client(`http://localhost:${address.port}`)
        .request({
            method: "GET",
            path: "http://example.com",
            headers: { host: "example.com" },
        })
        .then((res) => res.body.text()),
);

// This doesn't work and creates an infinite loop trying to connect to proxy url
// For instance, listening to server.on("connection", console.log) logs infinite times
console.log(
    await request("http://example.com", {
        dispatcher: new ProxyAgent(`http://localhost:${address.port}`),
    }).then((res) => res.body.text()),
);

Now here there are two questions:

  1. Why is the connection closing in the first place? Spent hours debugging but honestly couldn't find the reason
  2. Why is it looping until a connection is established? This creates a significant overhead both on the client and on the proxy server that is flooded by requests

The error is caught here where there is already something about "avoiding a loop in client.js#connect"

} catch (err) {
if (err.code === 'ERR_TLS_CERT_ALTNAME_INVALID') {
// Throw a custom error to avoid loop in client.js#connect
callback(new SecureProxyConnectionError(err))
} else {
callback(err)
}
}

Reproducible By

Example above

Expected Behavior

If the connection is closed, the error should be thrown immediately instead of trying infinite times.
Also, I think the example above should be successful considering that using a Client works

Logs & Screenshots

The error (added throw err to avoid loop)

SocketError: other side closed
    at Socket.onHttpSocketEnd (D:\-\node_modules\undici\lib\dispatcher\client-h1.js:902:22)
    at Socket.emit (node:events:530:35)
    at endReadableNT (node:internal/streams/readable:1698:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:90:21) {
  code: 'UND_ERR_SOCKET',
  socket: {
    localAddress: '::1',
    localPort: 60534,
    remoteAddress: '::1',
    remotePort: 60533,
    remoteFamily: 'IPv6',
    timeout: undefined,
    bytesWritten: 73,
    bytesRead: 0
  }
}

Environment

Node v22, all undici versions with ProxyAgent

Additional context

Using ProxyAgent with a "real" proxy works (even if it seems a bit slow?), so I'm not sure what is it expecting from my custom server that is different from a normal proxy

@DTrombett DTrombett added the bug Something isn't working label Nov 28, 2024
@metcoder95
Copy link
Member

Tunneling connection is not being made correctly. The server is tearing down the connection on every attempt, causing the agent to attempt to connect once more without success (due to design).

I'd recommend you to check the proxy npm package to have a fully fledge example. (I just debug it, didn't replicate yet what was causing the connection to be drop)

@DTrombett
Copy link
Contributor Author

Yeah, I know that my example proxy is not really complete but it's curious that the connection is retried every time, without even a timeout

@metcoder95
Copy link
Member

That's a good catch, actually its something that will be interesting to address; as what happens is that, by design, undici detects that there's a request still pending, and as the client has not been marked as destroyed yet, nor the request has failed or aborted, it loops attempting to connect to the downstream.

@ronag this was made intentionally by design?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants