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.

Sign up for GitHub

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

don't drop CloseEvent silently #12

Closed
wants to merge 5 commits into from
Closed

don't drop CloseEvent silently #12

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8c8a1da
log close event in console and decode reason number
cryptix Apr 22, 2015
4d974f6
use map[int]string for closeError code to string lookup
cryptix Apr 25, 2015
c6676be
remove some console debugging
cryptix Apr 25, 2015
818872c
close error: naive stab at using io.Pipe()
cryptix Apr 25, 2015
ef58859
close error: don't concat emptystring to reason
cryptix Apr 30, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 86 additions & 36 deletions conn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"time"

"github.com/gopherjs/gopherjs/js"
"honnef.co/go/js/console"
"honnef.co/go/js/dom"
)

Expand All @@ -23,19 +24,57 @@ func beginHandlerOpen(ch chan error, removeHandlers func()) func(ev *js.Object)
}
}

// closeErrorMap maps Close frame status codes to their string explanation
// See http://tools.ietf.org/html/rfc6455#section-7.4.1
var closeErrorMap = map[int]string{
1000: "Normal closure, meaning that the purpose for which the connection was established has been fulfilled.",
1001: "An endpoint is \"going away\", such as a server going down or a browser having navigated away from a page.",
1002: "An endpoint is terminating the connection due to a protocol error",
1003: "An endpoint is terminating the connection because it has received a type of data it cannot accept (e.g., an endpoint that understands only text data MAY send this if it receives a binary message).",
1004: "Reserved. The specific meaning might be defined in the future.",
1005: "No status code was actually present.",
1006: "The connection was closed abnormally, e.g., without sending or receiving a Close control frame",
1007: "An endpoint is terminating the connection because it has received data within a message that was not consistent with the type of the message (e.g., non-UTF-8 [http://tools.ietf.org/html/rfc3629] data within a text message).",
1008: "An endpoint is terminating the connection because it has received a message that \"violates its policy\". This reason is given either if there is no other sutible reason, or if there is a need to hide specific details about the policy.",
1009: "An endpoint is terminating the connection because it has received a message that is too big for it to process.",
// Note that this status code is not used by the server, because it can fail the WebSocket handshake instead
1010: "An endpoint (client) is terminating the connection because it has expected the server to negotiate one or more extension, but the server didn't return them in the response message of the WebSocket handshake.",
1011: "A server is terminating the connection because it encountered an unexpected condition that prevented it from fulfilling the request.",
1015: "The connection was closed due to a failure to perform a TLS handshake (e.g., the server certificate can't be verified).",
}

// closeError allows a CloseEvent to be used as an error.
type closeError struct {
*dom.CloseEvent
}

func (e *closeError) Error() string {
var cleanStmt string
var (
cleanStmt string
reason string // cant override the event's reason
ok bool
)
if e.WasClean {
cleanStmt = "clean"
} else {
cleanStmt = "unclean"

if e.Reason != "" {
reason = e.Reason
} else {
reason, ok = closeErrorMap[e.Code]
if !ok {
reason = "Unknown reason"
}

if e.Code == 1010 {
reason += "\nSpecifically, the extensions that are needed are: " + e.Reason
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of having this switch here, consider having an unexported map of codes to reasons outside, and this can reference that map. Maybe it would be better?


}
return fmt.Sprintf("CloseEvent: (%s) (%d) %s", cleanStmt, e.Code, e.Reason)

return fmt.Sprintf("CloseEvent: (%s) (%d) %s", cleanStmt, e.Code, reason)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to use string concatenation here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that string concatenation would be better here.

}

func beginHandlerClose(ch chan error, removeHandlers func()) func(ev *js.Object) {
Expand Down Expand Up @@ -72,6 +111,7 @@ func Dial(url string) (*Conn, error) {
ch: make(chan *dom.MessageEvent, 1),
}
conn.initialize()
conn.Reader, conn.pw = io.Pipe()

openCh := make(chan error, 1)

Expand Down Expand Up @@ -101,6 +141,8 @@ func Dial(url string) (*Conn, error) {
return nil, err
}

go conn.receiveLoop()

return conn, nil
}

Expand All @@ -115,6 +157,10 @@ type Conn struct {
readBuf *bytes.Reader

readDeadline time.Time

io.Reader
pw *io.PipeWriter
closeErr *closeError
}

func (c *Conn) onMessage(event *js.Object) {
Expand All @@ -125,6 +171,11 @@ func (c *Conn) onMessage(event *js.Object) {

func (c *Conn) onClose(event *js.Object) {
go func() {
if ce, ok := dom.WrapEvent(event).(*dom.CloseEvent); ok {
c.closeErr = &closeError{CloseEvent: ce}
console.Error(c.closeErr.Error())
}

// We queue nil to the end so that any messages received prior to
// closing get handled first.
c.ch <- nil
Expand All @@ -147,7 +198,11 @@ func (c *Conn) initialize() {
// convenience funciton to dedupe code for the multiple deadline cases.
func (c *Conn) handleFrame(item *dom.MessageEvent, ok bool) (*dom.MessageEvent, error) {
if !ok { // The channel has been closed
return nil, io.EOF
if c.closeErr == nil {
return nil, io.EOF

}
return nil, c.closeErr
} else if item == nil {
// See onClose for the explanation about sending a nil item.
close(c.ch)
Expand All @@ -157,6 +212,31 @@ func (c *Conn) handleFrame(item *dom.MessageEvent, ok bool) (*dom.MessageEvent,
return item, nil
}

// receiveLoop fill's the writer end of the Conn's io.Pipe with data frames
// runs in its own goroutine
func (c *Conn) receiveLoop() {

for {
frame, err := c.receiveFrame(true)
if err != nil {
if err2 := c.pw.CloseWithError(err); err2 != nil {
console.Error("CloseWithError failed", err2)
}
return
}

receivedBytes := getFrameData(frame.Data)

_, err = io.Copy(c.pw, bytes.NewReader(receivedBytes))
if err != nil {
if err2 := c.pw.CloseWithError(err); err2 != nil {
console.Error("CloseWithError failed", err2)
}
return
}
}
}

// receiveFrame receives one full frame from the WebSocket. It blocks until the
// frame is received.
func (c *Conn) receiveFrame(observeDeadline bool) (*dom.MessageEvent, error) {
Expand Down Expand Up @@ -197,41 +277,11 @@ func getFrameData(obj *js.Object) []byte {
return []byte(obj.String())
}

func (c *Conn) Read(b []byte) (n int, err error) {
if c.readBuf != nil {
n, err = c.readBuf.Read(b)
if err == io.EOF {
c.readBuf = nil
err = nil
}
// If we read nothing from the buffer, continue to trying to receive.
// This saves us when the last Read call emptied the buffer and this
// call triggers the EOF. There's probably a better way of doing this,
// but I'm really tired.
if n > 0 {
return
}
}

frame, err := c.receiveFrame(true)
if err != nil {
return 0, err
}

receivedBytes := getFrameData(frame.Data)

n = copy(b, receivedBytes)
// Fast path: The entire frame's contents have been copied into b.
if n >= len(receivedBytes) {
return
}

c.readBuf = bytes.NewReader(receivedBytes[n:])
return
}

// Write writes the contents of b to the WebSocket using a binary opcode.
func (c *Conn) Write(b []byte) (n int, err error) {
if c.closeErr != nil {
return 0, c.closeErr
}
// []byte is converted to an (U)Int8Array by GopherJS, which fullfils the
// ArrayBufferView definition.
err = c.Send(b)
Expand Down