Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix a bug that you can not reconnect on wechat #724

Merged
merged 3 commits into from
Dec 6, 2017

Conversation

johnzeng
Copy link

@johnzeng johnzeng commented Nov 22, 2017

On wechat, if the socket is closed, the socketOpen mark is not set to false , so you might send data before the websocket is opened, and the following codes will result in error:

function sendSocketMessage (msg) {
  if (socketOpen) {
    wx.sendSocketMessage({
      data: msg
    })
  } else {
    socketMsgQueue.push(msg)
  }
}

BTW, why are we pushing msg into a msg queue when the socket is not opened? If we send some message to socket before mqtt connection is done, those messages will be send to server, will it result in error?

@codecov
Copy link

codecov bot commented Nov 23, 2017

Codecov Report

Merging #724 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #724   +/-   ##
=======================================
  Coverage   92.88%   92.88%           
=======================================
  Files           8        8           
  Lines         703      703           
  Branches      173      173           
=======================================
  Hits          653      653           
  Misses         50       50

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 13f0219...5d214f6. Read the comment docs.

@mcollina
Copy link
Member

mcollina commented Dec 6, 2017

I’m not really familiar with the wechat support. We can’t really test it, so it’s best effort. I really don’t have an answer for you :/.

@mcollina mcollina merged commit 3f9af07 into mqttjs:master Dec 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants