Map bindings to traces based on the trace __FILE__ and __LINE__ #203
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
|
@sh19910711 do you wanna take a look? |
Currently, we naively map exception bindings by the exception backtrace's indexes. This doesn't work as the backtraces and the bindings doesn't map one to one and we're most likely to be out of sync in a couple of trace indexes. With this change, I try to be smarter about the binding that corresponds to an exception backtrace index. The `ExceptionMapper` objects tries to map to a binding based on the __FILE__ and __LINE__ of the trace behind that index. It also hides this transformation in an interface similar to an `Array`, so we didn't needed to change much backend code and any frontend code. Right now, if you go and click through all the traces, you get the binding in most of the times. I can imagine we can still be off in exceptions with custom backtraces, but we'll be off only for a couple of bindings and not everything after those special traces. In any case, I think this can fix a lot of the `web-console doesn't work` cases for Rails 5.
gsamokovarov
force-pushed
the
exception-mapper
branch
from
4bee1b6 to
3a27cc9
Compare
|
I'll go ahead and merge it for a swift |
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.
Currently, we naively map exception bindings by the exception
backtrace's indexes. This doesn't work as the backtraces and the
bindings doesn't map one to one and we're most likely to be out of sync
in a couple of trace indexes.
With this change, I try to be smarter about the binding that corresponds
to an exception backtrace index. The
ExceptionMapperobjects tries tomap to a binding based on the FILE and LINE of the trace behind
that index. It also hides this transformation in an interface similar to
an
Array, so we didn't needed to change much backend code and anyfrontend code.
Right now, if you go and click through all the traces, you get the
binding right in most of the times. I can imagine we can still be off in
exceptions with custom backtraces, but we'll be off only for a couple of
bindings and not everything after those special traces.
In any case, I think this can fix a lot of the
web-console doesn't workcases for Rails 5.