search

crossbario / autobahn-js

WAMP in JavaScript for Browsers and NodeJS
http://crossbar.io/autobahn
MIT License
1.43k stars 228 forks source link

_protocol_violation gives `undefined` instead of Error to util.handle_error() #434

Open jayphelps opened 5 years ago

jayphelps commented 5 years ago

When there's a protocol violation it invokes the optionally registered error handler (eventually on_internal_error) however it's provided with undefined as the error rather than an Error object. This appears to be due to the fact that it is not using the new keyword on the Error() constructor, so it's just being called like a regular function, returning undefined.

_protocol_violation: https://github.com/crossbario/autobahn-js/blob/c7e81a38551c15ca98b13a648e6cf13eae5af4db/lib/session.js#L283-L286

Error: https://github.com/crossbario/autobahn-js/blob/c7e81a38551c15ca98b13a648e6cf13eae5af4db/lib/session.js#L112-L119

If my investigation appears correct, I'm happy to PR the fix if helpful.

p1ngod commented 3 years ago

We got the same exception in our software using autobahn-browser and investigated some time into this issue. The problem with the code above is not the missing keyword new - it is the duplicated naming of Error.

The Error definition as stated by @jayphelps in https://github.com/crossbario/autobahn-js/blob/master/packages/autobahn/lib/session.js#L111-L118 (moved to new file) is a really bad naming decision as it overwrites the built-in Error class from ES/NodeJS.

So as soon as a protocol error occurs, the _protocol_violation function will use the Error function from session.js which is obviously designed to represent an incoming WAMP ERROR (like https://wamp-proto.org/_static/gen/wamp_latest.html#subscribe-error) message instead of a JavaScript exception.