SyliusResourceBundle
SyliusResourceBundle copied to clipboard
Customizable error handler
Hey.
In case of any unhandled exception, the current behavior is print whole stracktrace into response's body. It's not what we want and there is no way to alter this behavior currently.
This PR adds new configuration option :error-handler
to http server, this handler is called for unhandled exceptions inside HTTP machinery. By default, it's old error-response
.
Hey! Some additional error handling changes were implemented here. The idea of having fully customizable error handler seems like a good addition but there are a few questions to ask first:
- should
error-handler
provide only sync interface? or it might be also async? - should it run on it's own thread pool? should there be a way to configure executor (e.g. as it's done for
continue-handler
)? - there probably should be a wrapper to catch errors when
error-handler
is applied (at least to close connection to avoid resources from leaking)
Let me think about this a bit more.
This PR has been requested multiple times to be merged on Aleph and tests prove the functionality is working as expected.
Regarding Oleksii's comments:
should error-handler provide only sync interface? or it might be also async?
We can provide a sync interface for now and adapt afterwards if requested.
should it run on it's own thread pool? should there be a way to configure executor (e.g. as it's done for continue-handler)?
The continue-executor
is a second parameter, we should be able to provide a error-executor
if requested.
there probably should be a wrapper to catch errors when error-handler is applied (at least to close connection to avoid resources from leaking)
Yes, probably! But I think it can be handled afterwards as it's not the case currently.
Rebased, fixed and approved on my side.