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

📝 [Proposal v3]: Add End() method to Ctx #3276

Open
3 tasks done
grivera64 opened this issue Jan 7, 2025 · 4 comments
Open
3 tasks done

📝 [Proposal v3]: Add End() method to Ctx #3276

grivera64 opened this issue Jan 7, 2025 · 4 comments

Comments

@grivera64
Copy link
Member

grivera64 commented Jan 7, 2025

Feature Proposal Description

As brought up and discussed in #3252 (comment), this issue proposes to add the End() method to Ctx in v3 to allow instant flushing and closing of an active connection.

While Ctx's Drop() method added in the issue mentioned above allows to close a connection before sending a response, this new End() method would be to flush the current response before closing the connection.

This feature would be useful for stopping middleware from controlling the connection after a handler further up the method chain. Some middleware (like fiberprometheus) will work with a response after calling c.Next(). In addition, Fiber's default error handler may also write to the response, while a user may want to handle an error in different ways from within specific handlers and only log detailed errors internally.

It also can be an alternative to simply using return nil at the end of a handler, as done in ExpressJS (see below).

Alignment with Express API

The Express API provides an equivalent method res.end():

app.get("/", (_, res) => {
  res.write("Hello")
  res.end();
});

Calling res.end() closes the connection after flushing the response.

HTTP RFC Standards Compliance

N/A

API Stability

ExpressJS uses res.end(), so the API will be unlikely to be changed to match Express.

In addition, the relying c.RequestCtx().Conn() function is also consistent, so it would only require minor changes if that method is updated. Flushing data to the connection is also consistent, so no API updates would be needed there either.

Feature Examples

app.Use(func (c fiber.Ctx) error {
  err := c.Next()
  if err != nil {
    log.Println("Got error: %v", err)
    return c.SendString(err.Error()) // Will be unsuccessful since the response ended below
  }
  return nil
})

app.Get("/hello", func (c fiber.Ctx) error {
  query := c.Query("name", "")
  if query == "" {
    c.SendString("You don't have a name?")
    c.End() // Closes the underlying connection
    return errors.New("No name provided")
  }
  return c.SendString("Hello, " + query + "!")
})

Checklist:

  • I agree to follow Fiber's Code of Conduct.
  • I have searched for existing issues that describe my proposal before opening this one.
  • I understand that a proposal that does not meet these guidelines may be closed without explanation.
@gaby
Copy link
Member

gaby commented Jan 7, 2025

@grivera64 Makes sense to me 👍

@gaby
Copy link
Member

gaby commented Jan 7, 2025

@efectn @ReneWerner87 Thoughts?

@ReneWerner87
Copy link
Member

Yeah sounds good

@gaby gaby added this to v3 Jan 8, 2025
@gaby gaby moved this to Todo in v3 Jan 8, 2025
@gaby gaby added this to the v3 milestone Jan 8, 2025
@grivera64 grivera64 self-assigned this Jan 8, 2025
@grivera64
Copy link
Member Author

Sounds good! I will work on a PR for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

No branches or pull requests

3 participants