Panic when calling AckErrorEvent.Error()

[falzm]

Hi

The code panics when trying to calling the Error() method on a AckErrorEvent instance (looks like the underlying ErrorObj error is nil):

panic: value method slack.RTMError.Error called using nil *RTMError pointer

goroutine 18 [running]:
panic(0x988720, 0xc420433980)
        /usr/local/Cellar/go/1.7.3/libexec/src/runtime/panic.go:500 +0x1a1
github.com/nlopes/slack.(*RTMError).Error(0x0, 0x1, 0xffffffffffff0101)
        <autogenerated>:112 +0xc5
github.com/nlopes/slack.(*AckErrorEvent).Error(0xc42040aa10, 0x9, 0xa509f4)
        /Users/marc/dev/abitbot/vendor/src/github.com/nlopes/slack/websocket_internals.go:91 +0x33
abitbot/source.(*SlackSource).Run(0xc4201c0190)
        /Users/marc/dev/abitbot/src/abitbot/source/slack.go:361 +0x9ae
created by main.main
        /Users/marc/dev/abitbot/src/cmd/george/main.go:134 +0x950

Maybe it would be better for the Error() method to return directly the error value instead of a string representation, or at least check that the ErrorObj error is non-nil before calling the Error() method on it?

[mkarlesky]

We started seeing this same issue this morning. Concurrently, Slack has been having major platform outage issues all day.

It appears that the issue is related to the JSON Slack is sending back for an ACK. It's {}. The unmarshalling in handleAck() in websocket_managed_conn.go was not handling empty JSON well and led to a nil panic.

@falzm After checking our vendored version of the library against what's up to date in the repo, we saw that the newer code is smarter about this. We're now experimenting with updating our version of nlopes/slack.

[doertedev]

Any updates on this issue? :(

[mkarlesky]

The latest code in the repo as of the time of my comment had proper protections for the issue we were experiencing. Once we updated our vendored library (it was almost a year old) the problem was solved.

[dvdplm]

@mkarlesky thank you for reporting back. I'm going to close this issue as fixed then.