Skip to content

Conversation

@magicmark
Copy link
Collaborator

There was a flaw in the errorHandler logic. Because we wrap everything with ensureError, any non-errors passed to errorHandler get serialized away, and we lose access to the underlying thing returned from the resource. In the case of a rejected promise (like a 2xx) we don't get access to the response object :(

Let's change this, so that errorHandler takes in any instead of error, so it can see the original thing returned from the resource.

@magicmark magicmark force-pushed the fix_error_handling_logic_to_support_custom_errors branch 4 times, most recently from aa53b50 to 0090829 Compare March 31, 2020 01:36
});
});

test('batch endpoint that throws errors', async () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this since it's the same test as

test('batch endpoint (multiple requests, default error handling)', async () => {

@magicmark magicmark force-pushed the fix_error_handling_logic_to_support_custom_errors branch from 0090829 to c0ff8d5 Compare March 31, 2020 01:46
Copy link
Contributor

@ryanruanwork ryanruanwork left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Shipit!

@magicmark magicmark merged commit 617e928 into master Mar 31, 2020
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.

3 participants