977020 - websocket send raises uncaught exception

Status: RESOLVED INVALID

(Core :: DOM: Core & HTML, defect)

Description

[Daniel Szabo]

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:28.0) Gecko/20100101 Firefox/28.0 (Beta/Release)
Build ID: 20140218122424

Steps to reproduce:

An online game is using WebSockets for communication. Error logs indicate that many times uncaught exceptions will be fired.

The code sending data to the WebSocket is surrounded by a try-catch, like this:

if(webSocket.readyState == WebSocket.OPEN) {
  try {
    webSocket.send(...);
  }
  catch(e) {
  }
}


Actual results:

Error condition will be not caught by the catch tag. Instead, window.onerror will be called with the uncaught exception.

Typical error messages:

uncaught exception: [Exception... "Unexpected error"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: blob:d9535a55-a2dd-438b-91b8-b6297c50f3a9 :: <TOP_LEVEL> :: line 5721"  data: no]
User-agent: Netscape Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0

uncaught exception: [Exception... "The connection does not exist"  nsresult: "0x804b000c (NS_ERROR_NOT_CONNECTED)"  location: "JS frame :: blob:01e29123-cc79-4cfc-a7b1-dd4f3752559f :: <TOP_LEVEL> :: line 14091"  data: no]
User-agent: Netscape Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0



Expected results:

Uncaught exceptions should not be fired in any case. All errors should be catch-able.

If the WebSocket is non-functional, an "INVALID_STATE_ERR" Error should be thrown if "The connection is not currently OPEN", according to the documentation:
https://developer.mozilla.org/en-US/docs/Web/API/WebSocket#send%28%29

Comment 1

[Boris Zbarsky [:bzbarsky]]

Is there a link to a testcase that reproduces the problem?

What does the source on and around lines 5721 and 14091 look like?

Comment 2

[Daniel Szabo]

Unfortunately, we cannot create a testcase that could reproduce the problem. However, it happens frequently at users.
The source "on and around lines 5721 and 14091 look like" the one in the original post.
The actual code in production is obfuscated, so line numbers/variable names vary, but the exception is always raised on the "webSocket.send(...)" line.
We can add additional code and collect log results if you tell what else to look for (conditions/object properties etc.).
The exception is caught in the window.onerror handler, and not by the catch instruction, so we don't know what happens to the javascript code being executed.

Comment 3

[Boris Zbarsky [:bzbarsky]]

> The exception is caught in the window.onerror handler, and not by the catch instruction

That's really weird.  While non-exception things do get sent to window.onerror in some cases, the messages in comment 0 are definitely exceptions.

It's possible to get NS_ERROR_NOT_CONNECTED when doing a send on an already-closed websocket, based on looking at that code.  But that would still throw an exception.

I assume you haven't been able to reproduce at all yourself?  It doesn't have to be a minimal testcase, as long as it reproduces the issue; then we could do some more serious logging (that involves environment variables, so you can't do it on users' machines so much).

Comment 4

[Daniel Szabo]

Re: window.onerror
I think it's the onerror handler because there is no stacktrace in the exception. Our catch handler in this case just would ignore the exception and assume the connection is broken.
Also, the onerror handler's args (url/line number) are undefined, else the formatting of the log entry would be different: double colons and square brackets are not produced by our logging code.

Re: NS_ERROR_NOT_CONNECTED
before each send() socket status is checked using "if(webSocket.readyState == WebSocket.OPEN)" in order to prevent access to closed sockets. Anyway, in 95% of the cases the error code is NS_ERROR_UNEXPECTED.

I can understand that a socket's status may change asynchronously and even if readyState indicates otherwise calling send() may fail. But that exception should be catchable in javascript.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> because there is no stacktrace in the exception.

You mean .stack?  That's expected for those exceptions, unfortunately.  They're being thrown by the DOM implementation, not the JS engine, and don't have a useful .stack so far.

Just to make sure we're on the same page: the messages from comment 0 came through as the first argument to your page's window.onerror, right?

> Also, the onerror handler's args (url/line number) are undefined

Yes, also expected for those exceptions.

Again, what confuses me is that exceptions are of course generally catchable.  This is where not having any way to reproduce is being a problem; I can't figure out what could be causing the behavior you're seeing.

Comment 6

[Daniel Szabo]

I double checked our code and it's possible that a JS exception will be thrown that is not caught by our handler, if the exception is a JS string and not a JS Error object.

According to Mozilla documentation at
 https://developer.mozilla.org/en-US/docs/Web/API/WebSocket#send%28%29
INVALID_STATE_ERR and SYNTAX_ERR may be thrown
According to W3C documentation at
 http://dev.w3.org/html5/websockets/#dom-websocket-send
InvalidStateError may be thrown

There is no type indication in either case, but based on "..ERR" and "..Error" one would assume an Error object with some detail message.
Anyway, I will update our code to look for string exception arguments too. I can report back in a few days if that solves the problem.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Ah, so...  The state of exceptions in the DOM is a bit of a mess, not least because there have been recent spec changes that are not uniformly implemented yet.

Specifically, what we throw in a lot of cases is an object that is not an Error instance.  It's not a string, but rather an instance of the DOMException interface as defined at http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#ID-17189187

The spec at http://dom.spec.whatwg.org/#exception-domexception redefines this to be a subclass of Error, not an interface, but as I said that's pretty new.  Bug 960508 tracks making that change.

This, by the way, is why I asked in comment 1 what your code looks like.  You said it looks like comment 0, but comment 0 is clearly catching _all_ exceptions, unconditionally.  So what _does_ the code actually look like?

Comment 8

[Daniel Szabo]

> This, by the way, is why I asked in comment 1 what your code looks like. 
> You said it looks like comment 0, but comment 0 is clearly catching _all_
> exceptions, unconditionally.  So what _does_ the code actually look like?

Yes, that's my bad. The _original_ code looks like that before being cross-compiled to Javascript and obfuscated.
Exception handling in the cross-compiler is inheritance based. That's why it didn't catch the non-Error exception.
It gets cross-compiled into something like:
catch(e) {
 if(e instanceof E1) { ... }
 else if(e instanceof E2) {...}
 else throw e;
}

Since the issue was Firefox specific (IE/Chrome just worked fine) and "Unexpected error" looked pretty scary I went the wrong direction. (readyState OPEN vs unexpected error)

Also, the fix is in production code for 2 days now so I can confirm the DOM exception will be caught, indeed.

Thanks for your patience.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Daniel, thanks for double-checking all that!

I'm going to resolve this, I guess, since bug 960508 covers the only real issue here.

Comment 10

[me]

I'm still getting a ton of NS_ERROR_UNEXPECTED errors from my users on websocket.send, even on FF 42. From what I understand, this should never happen - I expect all errors to fire via socket.onerror. Is this a bug?

Comment 11

[me]

Here's a screenshot of the frequency with which I see these errors: https://www.dropbox.com/s/cyad97ncsfofxcu/Screenshot%202015-11-17%2016.43.17.png

The errors occur on FF 38, 40, 41, and 42, mostly.

Comment 12

[Boris Zbarsky [:bzbarsky]]

You can expect NS_ERROR_UNEXPECTED from websocket.send() in various cases, looks like.  For example when doing send() on a closed websocket (i.e. after calling close()).

Per spec, it's not clear what should happen here; the spec seems to talk about throwing exceptions but doesn't specify what kind.  But it's certainly the case that per spec send() can throw exceptions.

Comment 13

[me]

According to https://developer.mozilla.org/en-US/docs/Web/API/WebSocket#send(), send is only supposed to throw INVALID_STATE_ERR or SYNTAX_ERR. Are the docs incorrect?

Comment 14

[Boris Zbarsky [:bzbarsky]]

I wouldn't trust the docs to match the spec necessarily.  If you want to figure out what the behavior is supposed to be, please read the spec.

That said, NS_ERROR_UNEXPECTED is clearly not an exception any spec would say to throw, so it's worth having a bug on file on aligning the exact exception we're throwing with the spec.  I filed bug 1225834 on that.