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

Fix #391: Remove instance check for MessageEvent on stream error. #392

Merged
merged 2 commits into from
Aug 2, 2019

Conversation

abuiles
Copy link
Contributor

@abuiles abuiles commented Aug 2, 2019

Using MessageEvent will throw an error if you are running the SDK on
Node (this API is not present in Node).

This was added in the TypeScript migration to match the expected
interface.

Checking the error instance shouldn't be a concern on the SDK but
the consumer, is up to the consumer what to do with an
error. Additionally we shouldn't be hiding errors just because they
don't match a given interface.

Using `MessageEvent` will throw an error if you are running the SDK on
Node (this API is not present in Node).

This was added in the TypeScript migration to match the expected
interface.

Checking the `error` instance shouldn't be a concern on the SDK but
the consumer, is up to the consumer what to do with an
error. Additionally we shouldn't be hiding errors just because they
don't match a given interface.
@abuiles abuiles requested a review from morleyzhi August 2, 2019 17:45
@abuiles abuiles merged commit 064e182 into master Aug 2, 2019
@abuiles abuiles deleted the fix-391 branch August 2, 2019 17:52
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.

2 participants