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 #7093

Closed
Elyahou opened this issue Apr 26, 2021 · 8 comments · Fixed by #10366 or #10374
Closed

Improve error handling #7093

Elyahou opened this issue Apr 26, 2021 · 8 comments · Fixed by #10366 or #10374

Comments

@Elyahou
Copy link

Elyahou commented Apr 26, 2021

Improve and allow a more consistent error handling

Hi,

The error handling in kong is somewhat inconsistent:

  • If the error come from nginx and is catched by the directive defined in the nginx configuration it will trigger the error handler response to the client (that will render body depending on mime types of the request)
  • If the error come from nginx and is not catched by the directive above, it will trigger the default nginx error page
  • If the error come from kong code, as in these examples it will generally just return a json response, no matter what the mime type of the request was...
  • IF the error come from kong.response.error call, it will respond with a body build depending on mime type

When kong is a installed as a gateway that serve html along with json API's, these json errors are perceived like a bug...

Furthermore, when the errors are built depending on the mime type of the request, it use some hardcoded templates. I think it will be good if the kong user can override these templates to allow customisation.

That cause us some difficulties as well, when kong serve website that return HTML and do not allow to serve a branded error page.

I tried to fix this in this PR (still draft) by systematically calling to kong.response.error function in kong code to allow consistent errors response and also added a error template file configuration to allow error template override.

I would like to receive some feedback on this

Thanks !

@bungle
Copy link
Member

bungle commented Apr 28, 2021

The error handling in kong is somewhat inconsistent

I agree. That has been on our TODO, but we haven't had it prioritized. Currently it depends a bit on which phase the code runs that what happens with errors. E.g. plugin that throws runtime exception or other error on header_filter phase will just end and close the connection without response. I think we should always catch these errors, and it seems in access phase we do. Thank you for starting this work.

@Elyahou
Copy link
Author

Elyahou commented Apr 28, 2021

Thank you @bungle, Do you think it can be done incrementally, the fix I propose here is to make the error that currently exists in kong consistent and customisable. I agree that handling other kinds of errors should be done as well, but it is a broader scope IMO.

@Elyahou
Copy link
Author

Elyahou commented May 4, 2021

@bungle Can you please approve tests workflow for this PR: #7087.

Also what do you think about my last message about fixing the existing inconsistencies like I did in this proposal?

@Elyahou
Copy link
Author

Elyahou commented May 12, 2021

Is there any news on this one ? This is a real issue in our side because we need to present a customized error page in case of kong error, maybe there is some workaround to do this ?

@Elyahou
Copy link
Author

Elyahou commented Jun 16, 2021

@bungle @fffonion Any news here ?

@Elyahou
Copy link
Author

Elyahou commented Jul 27, 2021

Still no news ?

@akira28
Copy link

akira28 commented Jan 20, 2022

We would need this feature to have a consistent error response across the entire exposed API, in particular we want to return a Problem Details object https://datatracker.ietf.org/doc/html/rfc7807 on every HTTP errors, and also hide internal Kong errors if possible.
@Elyahou did you find any workaround for this?

@samugi
Copy link
Member

samugi commented Feb 28, 2023

Reopening so this will be closed by #10374

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