pusher / pusher-js

Pusher Javascript library
http://pusher.com
MIT License
2.11k stars 374 forks source link

The object sent to the client for the connection error event is incorrect #732

Closed russellgilbert closed 1 year ago

russellgilbert commented 1 year ago

Do you want to request a feature or report a bug? Report a bug.

What is the current behavior? In the "Connection" section of the file README.md, it explains how to bind on "error" to watch for an account that's over its limits:

pusher.connection.bind( 'error', function( err ) {
  if( err.error.data.code === 4004 ) {

However, when I receive the "error" event, the "err" object does not contain a key named "error", so trying to access err.error.data.code produces an error. The "err" object does however contain err.data.code. I believe this is because of the function getCloseError() in src/core/connection/protocol/protocol.ts:

getCloseError: function (closeEvent) {
   if (closeEvent.code !== 1000 && closeEvent.code !== 1001) {
      return {
         type: 'PusherError',
         data: {
            code: closeEvent.code,
            message: closeEvent.reason || closeEvent.message
         }
      };
   }
   else {
      return null;
   }
}

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar. While the Pusher client is connected, disconnect your network connection. I can connect a VPN and it happens, also. After about 30 seconds, getCloseError() will be called and the error event will be sent to the client.

What is the expected behavior? When binding to "error", the "err" parameter received should match the documentation in README.md.

Which versions of Pusher, and which browsers / OS are affected by this issue? The problem in getCloseError() exists in the latest version of the client (8.1.0) and I found it as far back as at least version 2.0.7. The description of how to bind to the "error" event was added to README.md in version 4.3.0 and was the same then as it is now.

Did this work in previous versions of Pusher? If so, which? None that I know of.

Note 1: The description in README.md is the same as on the Connection page of the Pusher Docs, under the heading "Detecting Connection Limits". Because of this, and because that description has been there for so long, I would think it would be better to change the "err" object to match the documentation rather than to change the documentation to match the object. My guess is that many people are already running code that matches the current documentation but aren't aware of the problem.

Note 2: Just as a sidenote, in the description from README.md, there's also a typo in the phrase, "...your account being over its' limits." (apostrophe not needed)

fbenevides commented 1 year ago

Fixed on #739 Thanks!