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

MessageEvent is not defined in Node, breaking streaming. #391

Closed
abuiles opened this issue Aug 2, 2019 · 0 comments
Closed

MessageEvent is not defined in Node, breaking streaming. #391

abuiles opened this issue Aug 2, 2019 · 0 comments
Assignees
Labels

Comments

@abuiles
Copy link
Contributor

abuiles commented Aug 2, 2019

Describe the bug
The onerror function defined in .stream checks if the received error is an instance of MessasgeEvent - this works fine in the browser but it will fail if you are running in node, since MessageEvent is not part of Node's API.

if (options.onerror && error instanceof MessageEvent) {

What version are you on?
2.2.0

To Reproduce
Steps to reproduce the behavior:

  1. git clone js-stellar-canary
  2. cd js-stellar-canary
  3. yarn
  4. node --experimental-modules src/streaming.js

Expected behavior
If there is an error from the stream, then it should be handled by my onerror function, but instead we get

/js-stellar-canary/node_modules/stellar-sdk/lib/call_builder.js:75
                    if (options.onerror && error instanceof MessageEvent) {
                                                            ^

ReferenceError: MessageEvent is not defined
    at EventSource.es.onerror (/js-stellar-canary/node_modules/stellar-sdk/lib/call_builder.js:75:61)
    at EventSource.emit (events.js:203:13)
    at _emit (/js-stellar-canary/node_modules/eventsource/lib/eventsource.js:242:17)
    at onConnectionClosed (/js-stellar-canary/node_modules/eventsource/lib/eventsource.js:52:5)
    at IncomingMessage.<anonymous> (/js-stellar-canary/node_modules/eventsource/lib/eventsource.js:169:9)
    at IncomingMessage.emit (events.js:208:15)
    at endReadableNT (_stream_readable.js:1154:12)
    at processTicksAndRejections (internal/process/task_queues.js:77:11)

Additional context
This error was originally reported by @danieldeusing on keybase

@abuiles abuiles added the bug label Aug 2, 2019
@abuiles abuiles self-assigned this Aug 2, 2019
abuiles added a commit that referenced this issue Aug 2, 2019
abuiles added a commit that referenced this issue 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.
@abuiles abuiles closed this as completed in 064e182 Aug 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant