fix(errors): Custom RxJS errors now all have a call stack #5686
+844
−157
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
benlesh
commented
Sep 1, 2020
| @@ -0,0 +1,15 @@ | |||
| /** @prettier */ | |||
|
|
|||
| export function createErrorClass<T>(name: string, setup: (this: Error, ...args: any[]) => void): T { | |||
There was a problem hiding this comment.
I think I'm going to add a doc comment about why this exists.
benlesh
commented
Sep 1, 2020
|
|
||
| /** @test {concat} */ | ||
| describe('concat operator', () => { | ||
| let rxTest: TestScheduler; | ||
|
|
||
| beforeEach(() => { | ||
| rxTest = new TestScheduler(assertDeepEquals); | ||
| rxTest = new TestScheduler(observableMatcher); |
There was a problem hiding this comment.
NOTE: These had to be changed, because a simple deep-equals check fails for expect(new CustomError()).to.deep.equal(new CustomError()) once call stacks are in the mix.
kolodny
approved these changes
Sep 2, 2020
| const MySpecialError: any = createErrorClass( | ||
| (_super) => | ||
| function MySpecialError(this: any, arg1: number, arg2: string) { | ||
| _super(this); |
There was a problem hiding this comment.
tiny nit: would it be cleaner to call _super(this, 'Super special error') instead?
There was a problem hiding this comment.
_super is probably a bad name, I'm just not sure what else to call it. :\
NOTE: Because we have to workaround compilation-to-ES5 inadequacies until call stack locations will show as being inside of the constructor implementation at the top level. This will go away as we move the community to ES2015+ BREAKING CHANGE: Tests that are written with naive expectations against errors may fail now that errors have a proper `stack` property. In some testing frameworks, a deep equality check on two error instances will check the values in `stack`, which could be different. fixes ReactiveX#4250
- Adds tests
benlesh
force-pushed
the
Issue-4250-errors-missing-stack-traces
branch
from
3a8e531
to
ac7aa52
Compare
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.
NOTE: Because we have to workaround compilation-to-ES5 inadequacies until call stack locations will show as being inside of the constructor implementation at the top level. This will go away as we are able to move the community to ES2015+
BREAKING CHANGE: Tests that are written with naive expectations against errors may fail now that errors have a proper
stackproperty. In some testing frameworks, a deep equality check on two error instances will check the values instack, which could be different.fixes #4250