Allows to change the default ErrorHandler response type #11522
Conversation
I think URI Violations happen after all of the headers are fully parsed, and before the dispatch to the handler tree. |
@joakime But for some reason in the ErrorHandler the request headers are empty. I created an issue with test case for that. However, independently of that, I thought it would be great to be able to change the default response content type for the error handler, in case a client does not specify any Accept header. |
| protected Type defaultResponseMimeType() { | ||
| return Type.TEXT_HTML; | ||
| } | ||
|
|
There was a problem hiding this comment.
I don't mind the idea of being able to configure a default response content type.
Please:
- Make a field
defaultResponseContentType, withpublicgetters and setters (so there is no need to extendErrorHandler). - Write a test case in
org.eclipse.jetty.server.ErrorHandlerTest.
Let us know if you are willing to do the above.
Thanks!
There was a problem hiding this comment.
Sounds good. I can look into that.
There was a problem hiding this comment.
@sbordet I applied & pushed the changes. Hope that is what you meant :)
| @@ -74,6 +74,7 @@ public class ErrorHandler implements Request.Handler | |||
|
|
|||
| boolean _showStacks = true; | |||
| boolean _showMessageInTitle = true; | |||
| Type _defaultResponseMimeType = Type.TEXT_HTML; | |||
There was a problem hiding this comment.
Please use String, so that it will be easier to configure also from XML.
There was a problem hiding this comment.
Type is changed to String
| */ | ||
| public void setDefaultResponseMimeType(Type defaultResponseMimeType) | ||
| { | ||
| _defaultResponseMimeType = defaultResponseMimeType; |
There was a problem hiding this comment.
| _defaultResponseMimeType = defaultResponseMimeType; | |
| _defaultResponseMimeType = Objects.requireNonNull(defaultResponseMimeType); |
There was a problem hiding this comment.
Type is changed to String and also uses the requireNonNull helper now
| "GET / HTTP/1.1\r\n" + | ||
| "Host: Localhost\r\n" + | ||
| "\r\n"); |
There was a problem hiding this comment.
Haven't used Java in years ;-) Nice that that is possible. I updated the test.
… it can be configured through XML
|
@sbordet I applied the requested changes, let me know if anything else :) |
|
@dhs3000 Thanks! |
Currently, the server returns HTML responses by default. It would be great to easily change this without having to overwrite the whole error handler.
The error handler tries to find the right/accepted response type using the
ACCEPTheader, but in some cases the header is not yet there when an error happens, e.g. with an URI violation, it seems the request is not yet fully parsed and therefore the requested content type not recognised.