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

🚀 Improve error handling for net error(s) #2421

Merged
merged 6 commits into from
Apr 21, 2023

Conversation

ReneWerner87
Copy link
Member

@ReneWerner87 ReneWerner87 commented Apr 17, 2023

Fixes: reverse proxy support #2419

With this change we want to improve the default error handler, so that it does not give out the internal network information of the dial in case of net.error.
So that fiber becomes more secure.

fixes: reverse proxy support #2419
@ReneWerner87 ReneWerner87 changed the title improve error handling for net error(s) 🚀 Improve error handling for net error(s) Apr 17, 2023
@ReneWerner87 ReneWerner87 linked an issue Apr 17, 2023 that may be closed by this pull request
3 tasks
@ReneWerner87
Copy link
Member Author

Copy link
Member

@leonklingele leonklingele left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really always an 502 Bad Gateway error?

app.go Outdated Show resolved Hide resolved
@gaby
Copy link
Member

gaby commented Apr 18, 2023

Is it really always an 502 Bad Gateway error?

I'm wondering the same thing 🤔

Co-authored-by: leonklingele <[email protected]>
@ReneWerner87
Copy link
Member Author

just give me good suggestions and i will change it

before it was always a 500 that was not specific and exposed the error message with the network information.

so i would either take the 500 again or 502 or 503

@gaby
Copy link
Member

gaby commented Apr 18, 2023

just give me good suggestions and i will change it

before it was always a 500 that was not specific and exposed the error message with the network information.

so i would either take the 500 again or 502 or 503

Is it possible to get the return code from the NetError object?

@ReneWerner87
Copy link
Member Author

not really, because it is not a http error
its a network connection error when trying the client request internally

@gaby
Copy link
Member

gaby commented Apr 19, 2023

@ReneWerner87 This is what GPT-4 says:

This function appears to be a default error handler for some web application, and it is handling errors and returning appropriate HTTP status codes. Given the provided code, it makes sense to return a 500 status code (Internal Server Error) by default.

The function first checks if the error is a net.Error, and if it is, it sets the error to ErrBadGateway. This would typically correspond to a 502 status code (Bad Gateway). Then, the function checks if the error is of type *Error, and if it is, it retrieves the status code from that error.

However, it is worth noting that the function does not explicitly return 503 (Service Unavailable) at any point. To include that, you might want to add another condition to check for specific errors corresponding to a 503 status code.

In summary, based on the provided code, it makes sense for the function to return 500 (Internal Server Error) by default, and 502 (Bad Gateway) if the error is a net.Error. To return a 503 (Service Unavailable), you would need to add additional logic to handle that case.

I guess bad gateway makes sense in terms of a reverse proxy.

@ReneWerner87
Copy link
Member Author

ReneWerner87 commented Apr 19, 2023

  • add an unittest

@ReneWerner87 ReneWerner87 merged commit 9feaf22 into master Apr 21, 2023
@efectn efectn deleted the improve_error_handling_net_errors branch May 8, 2023 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cloudflare and reverse proxy support
3 participants