Skip to content

Retry handler destroys the socket on error status code #4222

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

Open
fatal10110 opened this issue May 15, 2025 · 2 comments · May be fixed by #4228
Open

Retry handler destroys the socket on error status code #4222

fatal10110 opened this issue May 15, 2025 · 2 comments · May be fixed by #4228
Labels
bug Something isn't working

Comments

@fatal10110
Copy link
Contributor

Bug Description

On getting status code >= 400 from serever, retry handler throws an error which closes the open connection

Reproducible By

you will see here

New connection
connection closed
New connection
Hello World

const fastify = require("fastify");
const { RetryAgent, Agent } = require("undici");

const app = fastify();

let tries = 0;

app.get("/", (req, res) => {
  if (++tries % 2 !== 0) {
    return res.status(500).send("Test error");
  }

  res.send("Hello World");
});

app.server.on("connection", (socket) => {
  console.log("New connection");

  socket.on("close", () => {
    console.log("connection closed");
  });
});

async function start() {
  await app.listen({ port: 3000 });

  const a = new RetryAgent(new Agent({ connections: 1 }));
  const res = await a.request({
    origin: "http://localhost:3000",
    path: "/",
    method: "GET",
  });
  console.log(await res.body.text());
}
start();

Expected Behavior

It will pass the error but will leave the connection open if its non-connection related error

Additional context

By destroying the socket it also prevents the user get the error body from response

@fatal10110 fatal10110 added the bug Something isn't working label May 15, 2025
@mcollina
Copy link
Member

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

@fatal10110
Copy link
Contributor Author

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

I prepared some code, hopefully it is the correct fix, will send the PR soon

@fatal10110 fatal10110 linked a pull request May 17, 2025 that will close this issue
7 tasks
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

Successfully merging a pull request may close this issue.

2 participants