Guard against exceptions in to_s when cleaning objects #591
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.
Goal
When sending notifications we run through a "cleaning" stage which does things like filter out any keys that we shouldn't send in the error report (e.g. passwords). This calls
to_s
in a couple of different places, which can cause issues as objects are free to implement any logic and can therefore throw exceptions or infinitely recurse, which means we end up crashing while trying to cleanup the objectIn this PR we'll now recover from exceptions and stack overflows by replacing the values with one of our existing replacement strings:
[RECURSION]
if we get aSystemStackError
when stringifying a plain object[FILTERED]
if we get an exception when attempting to filter keys in a hash map — if an exception happens we can't tell if we should filter the key, so we have to assume we need toThis means the error report will have less information that intended, but this should be rare and if it does happen then the alternative is no report at all
Changeset
Changed
SystemStackError
as well as other exceptions when callingto_s
on an objectStandardError
andSystemStackError
when callingto_s
on a key when filtering a hash mapTests
Tests have been added for:
to_s
to_s
to_s
Discussion
Alternative Approaches
Outstanding Questions
Linked issues
Related to #577
Review
For the submitter, initial self-review:
For the pull request reviewer(s), this changeset has been reviewed for: