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

Show non 500 default NGINX error pages for various upstream errors #158

Merged
merged 1 commit into from
May 5, 2020

Conversation

jon-betts
Copy link
Contributor

@jon-betts jon-betts commented May 4, 2020

Currently this uses try_files but we don't actually have any of them. This means we get the NGINX fallback page instead with the right code.

In future we can add nicer pages.

Testing notes

The mapping is pretty clear from the nginx.conf file in the diff. Pick any code from the left and you should end-up with the code listed in the named section.

You can test this by:

  • Running make services
  • Open: http://localhost:9083/proxy/static/http://httpbin.org/status/<STATUS CODE HERE>
  • Previously: You would see the httpbin.org page with the relevant status code, or your browser interpretation of the error
  • Now: You should see the default NGINX page with the relevant code

@jon-betts jon-betts added the bug label May 4, 2020
@jon-betts jon-betts self-assigned this May 4, 2020
Currently this uses `try_files` but we don't actually have any of
them. This means we get the NGINX fallback page instead with the
right code.

In future we can add nicer pages.
error_page 404 410 = @proxy_not_found;
error_page 420 429 = @proxy_too_many_requests;
error_page 400 401 402 403 405 406 407 408 409 411 412 413 414 415 416 417 418 422 423 424 425 426 428 431 444 449 450 451 = @proxy_client_error;
error_page 500 501 502 503 504 505 506 507 508 509 510 511 598 599 = @proxy_upstream_error;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some attempt has been made to split out different cases we might find interesting. In particular "too many requests" type errors could be very interesting to us, and would be otherwise mixed in with other 400s. 404's also seem like a useful thing to reply to the user.

As far as I can tell, there's no way of using pattern matching here to say 5**. So we have to list everything we can think of.

@@ -60,6 +67,26 @@ http {
add_header "X-Via" "static-proxy, redirect";
}

location @proxy_not_found {
# Not found / gone => 404 not found
try_files /proxy/not_found.html =404;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These files don't have to exist yet. NGINX will try them, not find them and fallback to internal defaults.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to proxy 404s? I guess why not--these won't be considered as "errors" by monitoring services like Elastic Beanstalk's, they shouldn't cause any false alarms in monitoring or alerting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think 404 is one of the few HTTP codes which are understood by lay-people, so it's probably a good idea.

@jon-betts jon-betts merged commit b7277a7 into master May 5, 2020
@jon-betts jon-betts deleted the stop-proxying-errors branch May 5, 2020 10:40
@jon-betts jon-betts linked an issue May 5, 2020 that may be closed by this pull request
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 this pull request may close these issues.

Implement Nginx response status handling
2 participants