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]: Nil response being code 200 is immensily missleading. Use 500 instead #3332

Open
3 tasks done
GustavBW opened this issue Feb 26, 2025 · 3 comments
Open
3 tasks done

Comments

@GustavBW
Copy link

GustavBW commented Feb 26, 2025

Feature Proposal Description

Hi,
This proposal is short:
In Golang, I've gotten the impression that if a function that may an error, returns nil, all is well.

However, for a given middleware function, as of Fiber v2.52.5, that may be confused for just any other function that may return an error based on the documentation of fiber.App.Use():

app.Use(func(c *fiber.Ctx) error {
     return c.Next()
})

Returning nil by accident, is not handled very well.
There is no documentation of what a nil return means for middleware, as far as I'm aware, however I would kindly suggest that you ensure that the resulting response is anything but 200.

Given that you intend people to always return c.Next(), you could also make a nil return go c.Next() by default.
However, if returning nil is an error, kindly make sure to log an error or just provide any kind of feedback, that fiber.App.Use is being used incorrectly.

I've spend too long debugging this. I would like for others to be spared the experience.

Alignment with Express API

This has nothing to do with Express.js. If Express.js handles nil (null / undefined) returns in the same fashion, I'd suggest them to change as well.

HTTP RFC Standards Compliance

This does not concern RFC standards.

API Stability

This does not impact stability. Shouldn't be breaking either.

Feature Examples

My request is rather simple. If any further explanation is required, I'd be happy to elaborate.

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.
Copy link

welcome bot commented Feb 26, 2025

Thanks for opening your first issue here! 🎉 Be sure to follow the issue template! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

@ReneWerner87
Copy link
Member

ReneWerner87 commented Feb 26, 2025

We will check this and change the default response to 200 if no error is returned or the status code is not explicit touched/changed

@GustavBW
Copy link
Author

We will check this and change the default response to 200 if no error is returned or the status code is not explicit touched/changed

I have a feeling that it is because the default allocation of the Response object has status code set to 200 and is initialized when the context is (on request recieved i assume).
Changing it there would solve the root cause, and probably also ensure better feedback in other edge cases, but might be breaking.
I don't know if you've any way to tell whats a middleware and whats the final handler internally, but if you do, you could lazily instantiate the response object (through some kind of proxy) or right before the final handler.
That would make early returns (like auth) faster - shifting memory allocation further into the process, but probably also make the program crash on a nil return from middleware. Feedback is feedback i suppose.

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

No branches or pull requests

2 participants