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

Race condition: "Taking longer than usual to connect" #2655

Closed
Curiousite opened this issue Dec 3, 2019 · 2 comments · Fixed by #2656
Closed

Race condition: "Taking longer than usual to connect" #2655

Curiousite opened this issue Dec 3, 2019 · 2 comments · Fixed by #2656
Assignees
Labels
Bot Services Required for internal Azure reporting. Do not delete. Do not change color. bug Indicates an unexpected problem or an unintended behavior. customer-replied-to Required for internal reporting. Do not delete. customer-reported Required for internal Azure reporting. Do not delete. p0 Must Fix. Release-blocker
Milestone

Comments

@Curiousite
Copy link
Contributor

Version

NPM: "botframework-webchat": "^4.6.0"

botframework-webchat:bundle:variant = full
botframework-webchat:bundle:version = 4.6.0
botframework-webchat:core:version = 4.6.0
botframework-webchat:ui:version = 4.6.0

Describe the bug

When Web-chat loses connection to the server but connection is reestablished within 15 seconds (before the "Taking longer than usual to connect" message is shown) then the message will appear after the chat is connected again and never disappear.

The error appears to originate from a call to "sleep" within detectSlowConnectionSaga.js, where the RECONNECT_FULFILLED status in connectivitystatus.js is overwritten by CONNECT_STILL_PENDING, hence showing the message.

Steps to reproduce

  1. Establish a connection
  2. Go offline
  3. Reconnect within 15 seconds

Expected behavior

The slow-connection-warning-message should not appear after 15 seconds if connection is already reestablished.

@Curiousite Curiousite added Bot Services Required for internal Azure reporting. Do not delete. Do not change color. bug Indicates an unexpected problem or an unintended behavior. Pending customer-reported Required for internal Azure reporting. Do not delete. labels Dec 3, 2019
@cwhitten
Copy link
Member

cwhitten commented Dec 3, 2019

Hi @Curiousite. Thank you for the issue and the contribution! We'll review the pull request soon. See #2656 for updates.

@cwhitten cwhitten added the customer-replied-to Required for internal reporting. Do not delete. label Dec 3, 2019
@corinagum corinagum added QOL and removed Pending labels Dec 17, 2019
@cwhitten cwhitten added the R8 label Jan 9, 2020
@cwhitten cwhitten added the p0 Must Fix. Release-blocker label Jan 9, 2020
@cwhitten cwhitten removed their assignment Jan 9, 2020
@corinagum corinagum added this to the R8 milestone Jan 15, 2020
@compulim
Copy link
Contributor

I have repro-ed the issue in test and found the main cause in this code instead, detectSlowConnectionSaga.js.

The code did not take account into RECONNECT_FULFILLED or RECONNECT_REJECTED.

I am working on a proper fix and test case.

export default function* detectSlowConnectionSaga() {
  for (;;) {
    yield take([CONNECT_PENDING, RECONNECT_PENDING]);

    const connectivityRace = yield race({
-     fulfilled: take(CONNECT_FULFILLED),
+     fulfilled: take([CONNECT_FULFILLED, RECONNECT_FULFILLED]),
-     rejected: take(CONNECT_REJECTED),
+     rejected: take([CONNECT_REJECTED, RECONNECT_REJECTED]),
      slow: call(() => sleep(SLOW_CONNECTION_AFTER))
    });

    if ('slow' in connectivityRace) {
      yield put({ type: CONNECT_STILL_PENDING });
    }
  }
}

compulim added a commit that referenced this issue Feb 26, 2020
* fix(#2655): Race condition in activity status

* chore: add changelog entry

* fix: include defaultstate, to enable warning for initial connection

* Fix root cause

Co-authored-by: Constantin Hütterer <60255589+constantin-huetterer@users.noreply.github.com>
Co-authored-by: William Wong <compulim@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bot Services Required for internal Azure reporting. Do not delete. Do not change color. bug Indicates an unexpected problem or an unintended behavior. customer-replied-to Required for internal reporting. Do not delete. customer-reported Required for internal Azure reporting. Do not delete. p0 Must Fix. Release-blocker
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants