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

Error event emits without error object #138

Open
binarykitchen opened this issue Feb 17, 2018 · 29 comments
Open

Error event emits without error object #138

binarykitchen opened this issue Feb 17, 2018 · 29 comments

Comments

@binarykitchen
Copy link
Contributor

In my videomail client code this strange thing sometimes happens: no error object is emited along with the error event.

Like

stream.on('error', function (err) {
  console.log(err) // this will print undefined
})

Don't you agree, it should have an error object? An error without an error is not an error :)

@binarykitchen
Copy link
Contributor Author

Btw in the stack trace I can see this happens when queue is drained and Duplexify._destroy is called.

@mcollina
Copy link
Collaborator

Can you include that stacktrace?

@binarykitchen
Copy link
Contributor Author

i am afraid, due to wrong configs, browserify sourcemaps are broken on production, hence this stack trace is kinda cryptic and not very useful:

Error
    at Function.VideomailError.create (https://videomail.io/js/main-1000576986.min.js:3:906305)
    at null.<anonymous> (https://videomail.io/js/main-1000576986.min.js:3:971808)
    at https://videomail.io/js/main-1000576986.min.js:3:735480
    at EventEmitter.emit (https://videomail.io/js/main-1000576986.min.js:3:735605)
    at Duplexify._destroy (https://videomail.io/js/main-1000576986.min.js:3:727187)
    at https://videomail.io/js/main-1000576986.min.js:3:727047
    at Item.run (https://videomail.io/js/main-1000576986.min.js:3:778418)
    at drainQueue (https://videomail.io/js/main-1000576986.min.js:3:777238)
    at arlong:beforeload:6:321

@mcollina
Copy link
Collaborator

Let us know if you can reproduce this reliably and/or have a better stacktrace.

@binarykitchen
Copy link
Contributor Author

I'm afraid very hard to reproduce. Happens by random on a heavy traffic site.

Do you have a guess what this could be?

@mcollina
Copy link
Collaborator

Not really. Did it happen with the latest version of readable-stream? Can you try to roll that back?

@binarykitchen
Copy link
Contributor Author

how? not using readable-stream anywhere in the videomail-client code

@mcollina
Copy link
Collaborator

Duplexify is relying on readable-stream internally.

@binarykitchen
Copy link
Contributor Author

do you recommend to roll back readable-stream by rolling back websocket-stream? cos it's a nested-nested dependency

@mcollina
Copy link
Collaborator

Did this happen after a very recent deploy? I released a new version a week ago.

@binarykitchen
Copy link
Contributor Author

no, happened before as well

@mcollina
Copy link
Collaborator

then no, that's not the problem. We really need a stacktrace and/or a good way to reproduce.

@binarykitchen
Copy link
Contributor Author

ugh, this will be difficult for me.

is there a useful tool/package to inspect/record a stream of all its events/data it had till the error was emitted?

@Juul
Copy link

Juul commented Feb 22, 2018

As far as I can tell websocket-stream doesn't originate any errors itself but does propagate errors from the underlying WebSocket which of course is just a plain TCP socket, so your null errors could be either coming from the ws module or from the browser or from the operating system's socket implementation.

@binarykitchen
Copy link
Contributor Author

@Juul @mcollina ok now i always can reproduce this. and it is on iphones only:

  1. open www.videomail.io on safari
  2. allow camera, initiate connection blahblah
  3. hit the sleep button on your iphone
  4. let it sleep for at least one min
  5. unlock and it should reopen same site on safari
  6. now you see that weird error

i expect websocket-stream to resume but not to throw an error. or at least throw a descriptive error.

@mcollina
Copy link
Collaborator

I think this is a safari-mobile specific bug. If you can get a stacktrace, we can definitely generate a new error if it's not provided by the platform.

@binarykitchen
Copy link
Contributor Author

hard to get a stack trace here i am afraid - do you have any other examples running online producing valid stacktraces i could test from my iphone?

@mcollina
Copy link
Collaborator

You should be able to use Remote Web Inspector.

@binarykitchen
Copy link
Contributor Author

good idea. turns out it is an event object, not an error object itself.

this screenshot tells more:
screen shot 2018-02-28 at 13 21 40

when debugging deep down, looks like

  function onerror(err) {
    stream.destroy(err)
  }

is the starting point. and funny is that this references to the global Window where this.destroyed is undefined. i expect it to be destroyed when iphone is locked/sleeping.

@mcollina
Copy link
Collaborator

@binarykitchen let us know if you can send a fix!

@binarykitchen
Copy link
Contributor Author

dont know if i can.

socket.onerror <-- what instance is socket here in the browser?

its implementation has to be questioned here.

@mcollina
Copy link
Collaborator

it's a WebSocket

@binarykitchen
Copy link
Contributor Author

native implementation?

@mcollina
Copy link
Collaborator

the WebSocket from the browser.

@binarykitchen
Copy link
Contributor Author

okay, sounds like it's an ios bug of its websocket implementation ... where to start best?

@mcollina
Copy link
Collaborator

You should probably work around that in your code.

@binarykitchen
Copy link
Contributor Author

already have but then, what if other non-ios browsers truly emit an error instead of an event during stream errors? then my app code gets complicated just because of ios weird websocket error handling?

@mcollina
Copy link
Collaborator

unfortunately yes.

@binarykitchen
Copy link
Contributor Author

alright :(

... btw have reported to apple as well and will keep you posted here.

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

No branches or pull requests

3 participants