Skip to content
This repository was archived by the owner on Dec 3, 2021. It is now read-only.

fix: be defensive around use of window.WebSocket#24

Closed
keithamus wants to merge 1 commit intoappuri:masterfrom
keithamus:fix-be-defensive-around-use-of-window-websocket
Closed

fix: be defensive around use of window.WebSocket#24
keithamus wants to merge 1 commit intoappuri:masterfrom
keithamus:fix-be-defensive-around-use-of-window-websocket

Conversation

@keithamus
Copy link

This fixes #19, #23 by being more defensive around the use of window.WebSocket.

It also provides more informational errors when attempting to call .send() on a WebSocket that is not OPEN.

new Error('An attempt was made to use a WebSocket that is not usable'),
{ name: 'InvalidStateError' }
)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would we try to re-implement the WebSocket standard? calling realWs.send.apply(...) should just throw this error.

Copy link
Author

Choose a reason for hiding this comment

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

As previously stated - realWs.send is not always present

Copy link
Contributor

Choose a reason for hiding this comment

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

But that's not what this code is checking. If you want to check for th3 existence of send and throw an Error, that is acceptible.

}
})(function(global, navigator) {

var WebSocket = global.WebSocket
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't make sense. either a) WebSocket is native and supported and never replaced, or b) it is polyfilled and we shouldn't close over and force the users to understand ordering of this library and the polyfill.

Copy link
Author

Choose a reason for hiding this comment

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

The reason for this change is for another option: WebSocket has been overriden by browser extensions and users want to ensure that RobustWebSocket's implementation doesn't change from under them.

}
Object.defineProperty(RobustWebSocket, name, descriptor)
Object.defineProperty(RobustWebSocket.prototype, name, descriptor)
})
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be glad to take this change at robust-websocket's new home, nathanboktae/robust-websocket

@keithamus keithamus closed this Oct 5, 2018
@keithamus keithamus deleted the fix-be-defensive-around-use-of-window-websocket branch October 5, 2018 17:51
@keithamus
Copy link
Author

Closing this one in favour of nathanboktae/robust-websocket#2

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments