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

Improved error handling #32

Merged
merged 3 commits into from
Oct 25, 2023

Conversation

midahp
Copy link

@midahp midahp commented Sep 13, 2023

  • AppFinder will now return a 404 Not Found response instead of causing an exception if an app could not be found. A message will be logged with log level INFO
  • Exceptions handled by Horde_ErrorHandler will now return a reponse with status code 500 Internal Server Error. Previously it was 200
  • Refactored html creation logic in Horde_ErrorHandler
  • Implemented ErrorFilter middleware and added to top of middleware stack. Exceptions caused while using the middleware stack will now be handled by this class instead of Horde_ErrorHandler.
  • ErrorFilter middleware will return a json encoded response if the requests Accept header contains application/json. Otherwise the usual html error response will be returned.

@midahp
Copy link
Author

midahp commented Oct 25, 2023

@ralflang This is the PR I was talking about earlier

Copy link

@ralflang ralflang left a comment

Choose a reason for hiding this comment

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

In principle I am for it.

One objection: I would have put the BC break into /src and left the original /lib semantically unchanged but horde/core is a mess of glue stuff and at least it's changed consistently. I hope we don't break traditional error pages outside controller framework. Let's merge it.

@ralflang ralflang merged commit 036eabe into maintaina-com:FRAMEWORK_6_0 Oct 25, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants