Register logger middleware before errors middleware #16921
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
innerdvations
added
source: core:strapi
|
Hi @goodhoko good idea to change the order. We migrate the code to typescript recently so there is a conflict would you be willing to update your PR ? |
In the examples and project templates the "errors" middleware which turn thrown errors into HTTP responses is registered before the "logger" middleware. This causes any errors thrown in controllers to pierce through the logger middleware resulting in these requests not being logged. Eg. when a controller throws a ValidationError the resulting HTTP 400 request is not logged at all. Change the order of middleware registration so that the logger is 'above' the errors middleware and has a chance to log *all* requests.
goodhoko
force-pushed
the
log-error-responses
branch
from
9e61041
to
92bb176
Compare
|
@alexandrebodin There weren't any conflicts with |
|
Hmmm it seems the failed CI is just some flaky test. Can we rerun it? |
|
FYI we will need a small migration guide for this in the docs and probably to update the docs middleware section example. |
I don't think that one is you |
|
ok, is any of this actionable from my side? |
|
@Boegie19 here is the PR I was talking about |
|
This pr will also fix #13327 |
L-Weisz
pushed a commit
to L-Weisz/strapi
that referenced
this pull request
Jan 9, 2024
In the examples and project templates the "errors" middleware which turn thrown errors into HTTP responses is registered before the "logger" middleware. This causes any errors thrown in controllers to pierce through the logger middleware resulting in these requests not being logged. Eg. when a controller throws a ValidationError the resulting HTTP 400 request is not logged at all. Change the order of middleware registration so that the logger is 'above' the errors middleware and has a chance to log *all* requests. Co-authored-by: DMehaffy <derrickmehaffy@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the examples and project templates the "errors" middleware which turn thrown errors into HTTP responses is registered before the "logger" middleware. This causes any errors thrown in controllers to pierce through the logger middleware resulting in these requests not being logged.
Eg. when a controller throws a ValidationError the resulting HTTP 400 request is not logged at all.
Change the order of middleware registration so that the logger is 'above' the errors middleware and has a chance to log all requests.
The caveat to consider is that with this new order, when the "logger" middleware itself throws, the error won't be handled by the
errorsmiddleware so the resulting HTTP response may not adhere to the standard otherwise expected.