search

eclipse / paho.mqtt.javascript

paho.mqtt.javascript
Other
1.15k stars 467 forks source link

wireMessage.returnCode.indexOf is not a function #15

Closed jpwsutton closed 8 years ago

jpwsutton commented 8 years ago

migrated from Bugzilla #470096 status RESOLVED severity normal in component MQTT-JS for 1.2 Reported in version unspecified on platform PC Assigned to: James Sutton

Original attachment names and IDs:

On 2015-06-12 12:38:51 -0400, Karl Palsson wrote:

Seen with firefox 29. Works fine in newer versions of firefox, chromium and friends. Note that the hive mq websockets demo works just fine in firefox 29, and websockets support should be perfectly usable there, indeed, "caniuse.com" says I should have been able to use websockets all the way back to firefox 11.

AMQJS0005E Internal error. Error Message: wireMessage.returnCode.indexOf is not a function, Stack trace: Paho.MQTT</ClientImpl.prototype._handleMessage@http://localhost:8000/resources/paho/mqttws31.js:1302 Paho.MQTT</ClientImpl.prototype._on_socket_message@http://localhost:8000/resources/paho/mqttws31.js:1149 Paho.MQTT</scope/<@http://localhost:8000/resources/paho/mqttws31.js:157 : 5"

On 2015-06-15 07:52:40 -0400, Karl Palsson wrote:

Tested with released 1.0.1 and current git master, v1.0.1-4-g6ee5ef2. Both fail. Latest develop branch, v1.0.1-7-gb82756f also fails.

On 2015-06-15 08:03:35 -0400, Karl Palsson wrote:

The stack trace only shows line numbers, the problem area is the SUBACK case in _handleMessage,

wireMessage.returnCode.indexOf = Array.prototype.indexOf;

if (wireMessage.returnCode.indexOf(0x80) !== -1) {

On 2015-06-15 09:39:50 -0400, Karl Palsson wrote:

Created attachment 254438 workaround firefox indexOf problem

clearly however, this only works for subacks for a single subscription.

On 2015-06-15 09:42:05 -0400, Karl Palsson wrote:

After looking at this, I feel that the existing code was doing a poor job of correlating return codes with subscriptions anyway. Shouldn't it be sending a failure message only for the subscriptions that failed anyway? not just if there were any failures?

Or is this a sideaffect of the API issues outlined in https://bugs.eclipse.org/bugs/show_bug.cgi?id=440767 anyway?

On 2015-06-15 09:44:53 -0400, Karl Palsson wrote:

Finally, given that you can't actually subscribe to multiple topics in the same call anyway with the current api, the patch proposed is probably perfectly acceptable.

On 2015-06-15 09:49:11 -0400, Ian Craggs wrote:

James, can you take a look at this?

This page:

https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Array/indexOf

says that the function may not be supported on all browsers. It also gives an alternative implementation, for when the function is not available.

Options:

1) Ask for (Eclipse) permission to use the code snippet from the page above. 2) Reimplement without the Array.prototype.indexOf

(We should sort out the subscribing to multiple topics too)

Karl: I notice there are other calls to indexOf, on other objects. Presumably all other calls to indexOf methods work ok?

On 2015-06-15 10:09:17 -0400, Karl Palsson wrote:

I don't have an extensive test suite, but my own basic usage works with the patch I attached. I can subscribe, send a message, get messages. But I've not done any heavy testing beyond that.

What's strange is that Array.prototype.indexOf exists as a function in this browser. I just can't set any property on the returnCode object. Playing in the console produced this sort of behaviour

wireMessage.xxx = "blah" wireMessage.xxx <== "blah" wireMessage.returnCode.xxx = "blah" (no errors here) wireMessage.returnCode.xxx <== undefined. Array.prototype.indexOf <== function.....

Doesn't seem to actually be related to the indexOf call. Possibly related to the underlying Uint8Array being readonly somehow?

I think that without any changes for the multi subscription api, https://bugs.eclipse.org/bugs/show_bug.cgi?id=440767 the patch I've attached is just fine. There's only a possibility of a single rc right now anyway.

On 2015-07-21 05:14:08 -0400, James Sutton wrote:

I've implemented the attached fix in change 52241 as that will resolve the issue for now. When we implement multi-topic support for subscribe and unsubscribe, we will need to check that we don't break this again.