don't drop CloseEvent silently

[cryptix]

Hi,

currently the event object of the close event is silently ignored and dropped. I use a Conn from this package to build a net/rpc client and stumbled over this when I saw that uses of a closed connection returned an empty error. I can create a more verbose reproduction if needed.

Furthermore, the reason string was empty but the code was set (for me at least on Linux with Chromium42 and Firefox37). I found a post on stackoverflow which had compiled the code>reason mapping from RFC6455.

I think a proper fix should also fix or improve #7 but I wanted feedback first on how to progress on both cleanly.

[mjohnson9]

This is a good idea, as the current implementation provides no more information than EOF. I was actually thinking of switching to using an io.Pipe (it has Close and CloseWithError) now that they work.

I'll review the current changes.

@dominikh What do you think?

[mjohnson9]

You'll want to get rid of any console debugging

[cryptix]

I converted the string lookup to a map. It does look a lot cleaner.

I kept the one console.Error of the wrapped event in until we find a better way to expose it. io.Pipe sounds like a good solution.

[cryptix]

I tried io.Pipe() and it is functional but I don't get the close error exposed when using the Conn again. The error is still just an empty object.

Maybe I don't understand the receiveFrame() > handleFrame() flow fully.

I'll setup a simpler test app that is gopherjs serveable to get the sourcemaps working.

[dmitshur]

This line will only execute when e.Reason is the empty string. So you're effectively doing this:

reason += "\nSpecifically, the extensions that are needed are: " + ""

Perhaps this if e.Code == 1010 { block should be in the if e.Reason != "" { block instead?

[cryptix]

Oh right, that does nothing. Thanks @shurcooL.

Sadly I havn't seen a websocket implementation (on browser api level) that fills in those fields to acutally see how concatanating these would makes sense. In this case, if e.Reason has more than a list of exensions in it, this would get ugly. I'll remove the if e.Code == 1010 {} block until then.

[dmitshur]

What's the status here?

This PR seems quite out of date, should we close it, or is there anything to do here?

• It's based on old code that still imports dom package.
• There have been improvements and bug fixes in the gopherjs compiler that may make some workarounds no longer necessary.

[cryptix]

Agreed