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

HTTPException status is ignored if a Response is provided #2707

Closed
htunnicliff opened this issue May 18, 2024 · 3 comments · Fixed by #2767
Closed

HTTPException status is ignored if a Response is provided #2707

htunnicliff opened this issue May 18, 2024 · 3 comments · Fixed by #2767
Labels

Comments

@htunnicliff
Copy link

htunnicliff commented May 18, 2024

What version of Hono are you using?

4.3.7

What runtime/platform is your app running on?

Deno

What steps can reproduce the bug?

import { HTTPException, Hono } from "hono";

const app = new Hono();

// Should throw 400, status is 200
app.get("/example-a", (c) => {
  throw new HTTPException(400, {
    res: new Response("An exception", { status: 200 }),
  });
});

// Should throw 401, status is 200
app.get("/example-b", (c) => {
  throw new HTTPException(401, {
    res: c.text("An exception"),
  });
});

// Correctly throws 401
app.get("/example-c", (c) => {
  throw new HTTPException(401, {
    res: c.text("An exception", 401),
  });
});

app.onError((err, c) => {
  if (err instanceof HTTPException) {
    return err.getResponse();
  }

  console.error(`${err}`);
  return c.text("Internal Server Error", 500);
});

Deno.serve(app.fetch);

What is the expected behavior?

The behavior I expected is that the status provided to HTTPException would take precedent over response, especially if c.text(), c.json(), or c.html() are used, since those all set a 200 status by default.

What do you see instead?

The status of the response provided in the res argument always takes precedence.

Additional information

Unless this is intended behavior, I would propose HTTPException always return a response with the status code provided in the first argument.

Note: I did some cursory research into modifying the status of a Response instance. It appears modifying a Response status isn't possible, and clones of responses have read-only statuses as well. It would seem that implementing this change would necessitate creating a new Response, then applying the body and headers from the given Response, but ignoring the status and instead using the status given to HTTPException.

@yusukebe
Copy link
Member

Hi @htunnicliff

Thank you for raising the issue. This should be fixed, OR we should find another way to add a Response object to HTTPException. I'll consider it. Please wait a bit!

@htunnicliff
Copy link
Author

Great! Thank you!

If I think of a solution, I'll be sure to post here.

@htunnicliff
Copy link
Author

Thank you 🎉 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants