Log internal ActionDispatch::DebugExceptions errors #29131
Closed
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
|
r? @matthewd (@rails-bot has picked a reviewer for you, use r? to override) |
gsamokovarov
force-pushed
the
log-internal-errors-in-debug-exceptions
branch
from
7291858 to
7dcc1c7
Compare
maclover7
suggested changes
May 18, 2017
|
|
||
| # If we got to this point, the only thing that can crash is the | ||
| # render_exception method. As private as it is, it's monkey patched by a | ||
| # lot of plugins, that need to interact with application errors, |
There was a problem hiding this comment.
✂️ the comma between plugins and that
| @@ -66,7 +66,21 @@ def call(env) | |||
| response | |||
| rescue Exception => exception | |||
| raise exception unless request.show_exceptions? | |||
| render_exception(request, exception) | |||
|
|
|||
| # If we got to this point, the only thing that can crash is the | |||
There was a problem hiding this comment.
Should we try and pare this down a little bit, to just the essential? I feel like if people want the whole story, we should look at the commit for context 😬 😬
There was a problem hiding this comment.
Think so too, wrote it before the commit. I'll drop it, the PR is descriptive enough.
gsamokovarov
force-pushed
the
log-internal-errors-in-debug-exceptions
branch
from
7dcc1c7 to
4bf5532
Compare
What happens when your last resort error handling code crashes by itself? This is a good question. Currently, Rails tries really hard to at least show something to the browsers and not leave the end users with a blank page. For this to happen we have a carefully arranged chain of middlewares that can: - show developer friendly information about an error - show a generic fallback (`Something went wrong`) error response We do try really hard to make sure those middlewares don't crash, however sometimes they do. Most of the time, it's not even Rails' fault. For example the `ActionDispatch::DebugExceptions#render_exception`, as private as it is, it's commonly freedom patched by a lot of plugins, that need to interact with application errors, web-console included. The API of the render_exception method has changed in the past, and it can change in the future. This may be a common place of errors during upgrades. Previously, when `ActionDispatch::DebugExceptions` crashes, the default fallback error response defined in `ActionDispatch::ShowExceptions` kicks in. However, the internal error is not logged anywhere and this may throw a lot of Rails developers into limbo. What is going on? Have I done something? What's the error? For someone unfamiliar with the stack, this is quite the mysterious problem. We have had crashes in web-console and we have tried to tackle them with solutions like rails/web-console#181. This is a more general solution that can benefit all the plugins relying on overriding `#render_exception`. Now, when the plugins or `ActionDispatch::DebugExceptions` itself crashes, we will try very hard to log the internal error to the logs itself or the `stderr` if none of the standard logs have been initialized when the crash occurred.
gsamokovarov
force-pushed
the
log-internal-errors-in-debug-exceptions
branch
from
4bf5532 to
3639069
Compare
This was referenced Jun 22, 2017
Closed
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
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.
What happens when your last resort error handling code crashes by
itself? This is a good question. Currently, Rails tries really hard to
at least show something to the browsers and not leave the end users with
a blank page.
For this to happen we have a carefully arranged chain of middlewares
that can:
Something went wrong) error responseWe do try really hard to make sure those middlewares don't crash,
however sometimes they do. Most of the time, it's not even Rails' fault.
For example the
ActionDispatch::DebugExceptions#render_exception, asprivate as it is, it's commonly freedom patched by a lot of plugins,
that need to interact with application errors, web-console included.
The API of the render_exception method has changed in the past, and it
can change in the future. This may be a common place of errors during
upgrades.
Previously, when
ActionDispatch::DebugExceptionscrashes, the defaultfallback error response defined in
ActionDispatch::ShowExceptionskicks in. However, the internal error is not logged anywhere and this
may throw a lot of Rails developers into limbo. What is going on? Have
I done something? What's the error?
For someone unfamiliar with the stack, this is quite the mysterious
problem. We have had crashes in web-console and we have tried to tackle
them with solutions like rails/web-console#181. This is a more general
solution that can benefit all the plugins relying on overriding
#render_exception.Now, when the plugins or
ActionDispatch::DebugExceptionsitselfcrashes, we will try very hard to log the internal error to the logs
itself or the
stderrif none of the standard logs have beeninitialized when the crash occurred.