stomp-js / stompjs

Javascript and Typescript Stomp client for Web browsers and node.js apps
Apache License 2.0
773 stars 82 forks source link

Issue in Using with ReactNative, NULL chopping #89

Open vinayak2095 opened 5 years ago

vinayak2095 commented 5 years ago

I am trying to integrate STOMP on my reactNative Project... When I open my debug mode everything works fine but when i close my debug mode and run it normally it is not able to fetch data//

alert('Component did mount'); client.configure({ brokerURL: 'ws://x.x.x.x:xxxx/stomp', onConnect: () => { alert('onConnect');

    client.subscribe('topic', message => {
      console.log('show msg...',message);
      console.lo('show msg body...',message.body);

    });

    client.subscribe('topic', message => {
      console.log(message);

    });

  },

});

client.activate();
kum-deepak commented 5 years ago

Please enable console log for the library and attach it.

kum-deepak commented 5 years ago

Also see https://github.com/stomp-js/stompjs/issues/55

ericschaal commented 5 years ago

I am having the same issue after upgrading from react native 0.57.x to 0.58.6. Works perfectly when connected to the chrome debugger but doesn't work with without remote debugging.

ericschaal commented 5 years ago

I was able to obtain the following using safari debugging (iPhone X Simulator) None of the callbacks are called:

client.onConnect
client.onStompError
client.onWebSocketClose

[Log] [DEBUG][16:39:59:791][WSService]: Opening Web Socket... (index.bundle, line 887) [Log] [DEBUG][16:39:59:919][WSService]: Web Socket Opened... (index.bundle, line 887) [Log] [DEBUG][16:39:59:921][WSService]: >>> CONNECT (index.bundle, line 887) accept-version:1.0,1.1,1.2 heart-beat:2000,10000 [Log] [DEBUG][16:39:59:939][WSService]: Received data (index.bundle, line 887) [DEBUG][16:59:42:489][WSService]: <<< CONNECTED version:1.2 heart-beat:0,0

EDIT:

After a long debugging session here are my findings:

Adding console.log statements in the Parser.parseChunk function I realized that I am not receiving the same chunk with/without remote debugging enabled (V8 vs JavascriptCore?).

By adding

        if (chunk[chunk.length-1] !== 0) {
            var withNull = new Uint8Array(chunk.length + 1);
            withNull.set(chunk, 0);
            withNull[chunk.length] = 0;
            chunk = withNull;
        }

to Parser.parseChunk I was able to fix the issue and everything was working as with React Native pre 0.58. Not sure what is happening.

Hope this helps.

vinayak2095 commented 5 years ago

I was able to obtain the following using safari debugging (iPhone X Simulator) None of the callbacks are called:

client.onConnect
client.onStompError
client.onWebSocketClose

[Log] [DEBUG][16:39:59:791][WSService]: Opening Web Socket... (index.bundle, line 887) [Log] [DEBUG][16:39:59:919][WSService]: Web Socket Opened... (index.bundle, line 887) [Log] [DEBUG][16:39:59:921][WSService]: >>> CONNECT (index.bundle, line 887) accept-version:1.0,1.1,1.2 heart-beat:2000,10000 [Log] [DEBUG][16:39:59:939][WSService]: Received data (index.bundle, line 887) [DEBUG][16:59:42:489][WSService]: <<< CONNECTED version:1.2 heart-beat:0,0

EDIT:

After a long debugging session here are my findings:

Adding console.log statements in the Parser.parseChunk function I realized that I am not receiving the same chunk with/without remote debugging enabled (V8 vs JavascriptCore?).

  • Uint8Array(73)[67,79,78,.......10, 0] with remote debugging (V8, js running in chrome)
  • Uint8Array(72)[67,79,78,.......10] without remote debugging (js running on the phone) Notice the missing 0 byte.

By adding

        if (chunk[chunk.length-1] !== 0) {
            var withNull = new Uint8Array(chunk.length + 1);
            withNull.set(chunk, 0);
            withNull[chunk.length] = 0;
            chunk = withNull;
        }

to Parser.parseChunk I was able to fix the issue and everything was working as with React Native pre 0.58. Not sure what is happening.

Hope this helps.

I did this way ,But it didn't worked.

kum-deepak commented 5 years ago

The underlying reason is a bug in ReactNative's handling of strings. As per STOMP specifications each Frame is terminated by NULL. Reactive Native has this issue reported, however, marked inactive - https://github.com/facebook/react-native/issues/12731

The workarounds may lead to incorrect behavior, two cases:

You can use your workaround with understanding of above aspects.

The parseChunk method is in https://github.com/stomp-js/stompjs/blob/master/src/parser.ts

If you need workaround for sending messages, please see #55

vinayak2095 commented 5 years ago

The underlying reason is a bug in ReactNative's handling of strings. As per STOMP specifications each Frame is terminated by NULL. Reactive Native has this issue reported, however, marked inactive - facebook/react-native#12731

The workarounds may lead to incorrect behavior, two cases:

  • If the server sends more than one frame in a single WebSocket Frame. (allowed as per STOMP specs).
  • If server splits a Frame into multiple SENDs - (not sure if allowed by STOMP specs, but, used by some server, in particular Spring).

You can use your workaround with understanding of above aspects.

The parseChunk method is in https://github.com/stomp-js/stompjs/blob/master/src/parser.ts

If you need workaround for sending messages, please see #55

Thanks ,But Nothing worked for me.., I am getting this warning when my debug mode is close & unable to connect with stomp - " Possible unhandled promise rejection()id:0: Reference Error:Can't find variable:TextEncoder Parser@http://localhost:8081/index.bundle?........................" and when I open my debug mode ,I am not getting this warning....and then i am able to connect

slavikdenis commented 5 years ago

@vinayak2095 See 'NodeJS'/'TextEncoder/TextDecoder' section https://stomp-js.github.io/guide/stompjs/rx-stomp/ng2-stompjs/2018/06/28/pollyfils-for-stompjs-v5.html

But, this will help you only with the TextEncoder error. The problem described in this issue still occurs.

ericschaal commented 5 years ago

I was aware of issue https://github.com/facebook/react-native/issues/12731, but until 0.58 it seemed to only affect android. Not sure what changed.

kum-deepak commented 5 years ago

Will any of you be able to raise an issue in react-native?

ericschaal commented 5 years ago

I can take care of it. In the mean time, @kum-deepak do you think forcing spring to send binary frames would bypass this issue?

kum-deepak commented 5 years ago

@ericschaal, forcing Spring to always send binary frames might actually bypass the issue. Please try and check.

vinayak2095 commented 5 years ago

Still I am facing same issue..

slavikdenis commented 5 years ago

@ericschaal 's workaround works great. Could it be released ? Thanks

kum-deepak commented 5 years ago

I will release it as a flag switched off by default. Give me a few days.

slavikdenis commented 5 years ago

Great, thank you so much :)

kum-deepak commented 5 years ago

Released @stomp/stompjs@5.3.0-beta1, give it a try. Set appendMissingNULLonIncoming to true as part of config to activate the behavior.

slavikdenis commented 5 years ago

@kum-deepak works great for me :) thanks again

kum-deepak commented 5 years ago

Thanks for testing it out. I will wait for few days and then promote this release as 5.3.0.

slavikdenis commented 5 years ago

@kum-deepak seems like on a 'release build' it's still broken (I mean RN, not stompjs).

kum-deepak commented 5 years ago

I have release 5.3.0.

kum-deepak commented 5 years ago

@kum-deepak seems like on a 'release build' it's still broken (I mean RN, not stompjs).

What do you mean, even this build of stompjs fails with React Native in release build mode?

slavikdenis commented 5 years ago

@kum-deepak unfortunately yes, and its not working even on 'development build' when debugging is off. I can't figure out why. Only solution working for us right now both on development and release is @ericschaal 's (https://github.com/ericschaal/stompjs)

Just to declare, I'm creating the Client like:

      const client = new Client({
        brokerURL: `${wsUrl.toString()}${WS_POSTFIX}`,
        forceBinaryWSFrames: true,
        appendMissingNULLonIncoming: true,
        logRawCommunication: true,
        reconnectDelay: 0, // disable auto reconnect
        debug: str => {
          console.warn(str);
        },
      });
kum-deepak commented 5 years ago

Let me have a look. This fix is getting bit blind for me as I do not have a React Native setup. I will take https://github.com/ericschaal/stompjs code and put the logic under control of a flag.

slavikdenis commented 5 years ago

Thanks once again. I really appreciate your help.

iMitaka commented 5 years ago

Maybe you miss to pass the property to StompHandler on client.ts line 362 (develop branch), you need to explicit add appendMissingNULLonIncoming: this.appendMissingNULLonIncoming in config object :)

kum-deepak commented 5 years ago

Thanks @iMitaka, that might do the trick. I have updated code in issue89 branch (https://github.com/stomp-js/stompjs/commit/12141ad29349a6e010577bfa029cb585a65a5311).

Please install and test directly like the following:

$ npm i https://github.com/stomp-js/stompjs/raw/bin/stomp-stompjs-5.4.0.tgz

Many thanks for all your support :smile:

iMitaka commented 5 years ago

I test it, but for now still not working with this fix...

iMitaka commented 5 years ago

Why you do not try to add this line into parser.ts line 86 like @ericschaal did into there fix

if (chunk[chunk.length - 1] !== 0) { const chunkWithNull = new Uint8Array(chunk.length + 1); chunkWithNull.set(chunk, 0); chunkWithNull[chunk.length] = 0; chunk = chunkWithNull; }

you can pass the flag as second param of function parseChunk(segment: string|ArrayBuffer, appendMissingNULLonIncoming: boolean) and to check

if (appendMissingNULLonIncoming && chunk[chunk.length - 1] !== 0) { const chunkWithNull = new Uint8Array(chunk.length + 1); chunkWithNull.set(chunk, 0); chunkWithNull[chunk.length] = 0; chunk = chunkWithNull; }

then you can remove the code for FLAG from stomp_handler.ts and i hope will work this way :)

kum-deepak commented 5 years ago

I will try this next - over the weekend.

kum-deepak commented 5 years ago

@iMitaka will you be able to do a Pull Request, only include the changed .ts files.

kum-deepak commented 5 years ago

Thanks @iMitaka! I have updated a candidate tar ball. Please install and test directly like the following:

$ npm i https://github.com/stomp-js/stompjs/raw/bin/stomp-stompjs-5.4.0.tgz

I am intending to make public release on Monday. Many thanks for all your support!

iMitaka commented 5 years ago

I'll test it on Monday morning when i go to office as a first task!

iMitaka commented 5 years ago

I test it right now but problem is not solved, i check the node_modules js files but the new changes are not in code, it's your last build, not after my pull request, maybe npm get cashed version from first build of 5.4.0.tgz

iMitaka commented 5 years ago

try to change it to 5.4.1

kum-deepak commented 5 years ago

Uploaded just now. Try with:

$ npm i https://github.com/stomp-js/stompjs/raw/bin/stomp-stompjs-5.4.1.tgz
iMitaka commented 5 years ago

the package is updated now, but the fix is not working in my RN project

kum-deepak commented 5 years ago

I guess it works with your branch, if it does, we can provide a tgz version (by running npm pack) from there.

iMitaka commented 5 years ago

After big researches for this issue, we decide to try downgrade to version 4 of stomp and now everything work perfect, all issues are gone! :)

slavikdenis commented 5 years ago

@kum-deepak for me it worked great.

@iMitaka interesting... hope this helps:

I have tried this and worked: yarn add https://github.com/stomp-js/stompjs/raw/bin/stomp-stompjs-5.4.1.tgz

package.json after this look like: "@stomp/stompjs": "https://github.com/stomp-js/stompjs/raw/bin/stomp-stompjs-5.4.1.tgz",

Created a Client like this:

      const client = new Client({
        brokerURL: wsUrl,
        forceBinaryWSFrames: true,
        appendMissingNULLonIncoming: true,
      });

Also, my subscriptions are handled like this:

      client.onConnect = () => {
        const subscription = client.subscribe(`/topic/user/${wsUser.id}`, message => {
          const data = JSON.parse(message.body);
          // process data here
        });

        wsSubs.push(subscription);
      };

Everything works as expected :)

EndlezzCoding commented 5 years ago

I tried StompJs 5 and 4 both can't connect on React Native 0.59 on iOS Only works when enabled remote debugging. I have colleague using React Native 0.57 and StompJs 5 works on iOS. So I think there are some issue with the Javascript engine on 0.59+? Are you guys using React Native 0.59+?

kum-deepak commented 5 years ago

Please see comment https://github.com/stomp-js/stompjs/issues/89#issuecomment-469999400 for a background. A hacked solution at https://github.com/stomp-js/stompjs/issues/89#issuecomment-485392159

Please see if that works.

slavikdenis commented 5 years ago

@atominvention in the above process I indeed used React Native 0.59+ (0.59.5), but only tested on Android devices.

EndlezzCoding commented 5 years ago

I have tried stomp-stompjs-5.4.1.tgz on React Native 0.59.4 on iOS. It works when setting both config forceBinaryWSFrames: true and appendMissingNULLonIncoming: true

kum-deepak commented 5 years ago

I have been able to figure out way to simulate this error and target a test case for it. This will allow confidently restructure the code. Please check https://github.com/stomp-js/stompjs/commit/eaf2d2a1d9cf5303206b9eedfce6c3c67a13e7d6

Basically it is a wrapper around the WebSocket which chops everything after NULL to simulate the case.

kum-deepak commented 5 years ago

I have made general release of 5.4.1. Also wrote a small guide at https://stomp-js.github.io/workaround/stompjs/rx-stomp/ng2-stompjs/2019/06/10/react-native-null-chopping-issue.html

Smiche commented 5 years ago

If you're using ActiveMQ stomp as your broker you can fork the ActiveMQ repository and look at activemq-http/src/main/java/org/apache/activemq/transport/ws/StompWSConnection.java. Look at the onWebSocketText method and simply add the null char to the data string. Rebuild and swap the new jar file in your activemq installation folder under lib. It's a shame that react-native does that as NULL termination is a part of the Stomp spec.

MQTT on the other hand doesn't need that as it advertises message length as a header, however only versions prior to 5.0 are implemented with ActiveMQ and they miss user made headers.

And a final workaround would be to fork react-native and add the NULL char to the websocket implementation and use your custom fork for builds.

hdvreyes commented 3 years ago

The issue moved to #323