Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow skipping variable inspection by class name #449

Conversation

@felixbuenemann
Copy link
Contributor

felixbuenemann commented Feb 16, 2020

This adds a new global setting BetterErrors.ignored_classes that will skip inspecting the value of local and instance variables of this type.

The default setting ignores action dispatch request and response instances, which are usually not printed anyways due to exceeding the limit of BetterErrors.maximum_variable_inspect_size, but also avoids
all the memory allocations and database queries that are triggered by calling #inspect on variables.

The default setting was chosen to avoid excessive database queries for unloaded relations, because both the ActionDispatch::Request and the ActionDispatch::Response objects hold many references to the controller instance, causing the same instance variables to be inspected over and over again.
(See rails/rails#38445 for details.)

This is a safer alternative to the undocumented setting BetterErrors.ignored_instance_variables, which could be set to [:@_request, :@_response] for Rails. It also doesn't completely hide the variables, but instead shows a message why the value was omitted, similar to maximum_variable_inspect_size.

This adds a new global setting `BetterErrors.ignored_classes` that will
skip inspecting the value of local and instance variables of this type.
The default setting ignores action dispatch request and response
instances, which are usually not printed anyways due to exceeding the
limit of `BetterErrors.maximum_variable_inspect_size`, but also avoids
all the memory allocations and database queries that are triggered by
calling `#inspect` on variables.

The default setting was chosen to avoid excessive database queries for
unloaded relations, because both the `ActionDispatch::Request` and the
`ActionDispatch::Response` objects hold many references to the
controller instance, causing the same instance variables to be inspected
over and over again.
@felixbuenemann
Copy link
Contributor Author

felixbuenemann commented Feb 16, 2020

Seems like with the BOC specs there's a different string representation for inspect_value, which lacks the quotes. I'll rewrite the spec changes to work around that.

For some weird reason with binding of caller loaded the inspected value
for strings is no longer quoted, breaking the specs, so this changes
the specs back to using all symbols.
@RobinDaugherty
Copy link
Member

RobinDaugherty commented Mar 2, 2020

Now that the specs were fixed by #453, I changed the specs to use the new matching method.

I changed the specs to explicitly set up a context with an ignored class rather than changing all of the other contexts/examples to use a Symbol.

@RobinDaugherty RobinDaugherty merged commit a71a1ee into BetterErrors:master Apr 24, 2020
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.