Skip to content

Prevent FallbackScan from polluting exception cause#314

Merged
burke merged 1 commit intorails:masterfrom
aaronjensen:fix-error-cause-pollution
Aug 5, 2020
Merged

Prevent FallbackScan from polluting exception cause#314
burke merged 1 commit intorails:masterfrom
aaronjensen:fix-error-cause-pollution

Conversation

@aaronjensen
Copy link
Copy Markdown
Contributor

When debugging an issue with test-bench and bootsnap, I discovered that bootsnap pollutes error.cause by requiring the file naturally after a fallback scan within a rescue. This causes any error that is thrown within the required file to have a #cause defined, which it should not. This improves output in test-bench when an error occurs and it may (I haven't tested this) improve output in other frameworks that inspect error.cause.

I wasn't sure how to write a test for this because I didn't know how to trigger a fallback scan (I don't even really know what that is) and still load a real file that raises an exception. If you could give me a snippet for that I can add a test.

Thanks!

@aaronjensen aaronjensen requested a review from burke as a code owner July 31, 2020 03:57
@ghost ghost added the cla-needed label Jul 31, 2020
@sbellware
Copy link
Copy Markdown

The maintainer might just take this code as a proof-of-concept and make the changes themselves. That would obviate the CLA hoop-jumping and delays.

@aaronjensen aaronjensen force-pushed the fix-error-cause-pollution branch from 7c5f386 to 29b77ee Compare July 31, 2020 15:24
@ghost ghost removed the cla-needed label Jul 31, 2020
@burke
Copy link
Copy Markdown
Contributor

burke commented Aug 5, 2020

LGTM. We'll live without a test for this.

@burke burke merged commit b1096d1 into rails:master Aug 5, 2020
@aaronjensen aaronjensen deleted the fix-error-cause-pollution branch August 5, 2020 16:18
@sbellware
Copy link
Copy Markdown

Awesome ❤️

@aaronjensen
Copy link
Copy Markdown
Contributor Author

Thank you for the merge!

If you have a free moment to cut a new gem release, I'd really appreciate it. Thank you.

@burke
Copy link
Copy Markdown
Contributor

burke commented Aug 11, 2020

Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants