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 reconnection scenarios and various behaviors of Direct Line ASE #404

Merged
merged 25 commits into from
Apr 26, 2023

Conversation

compulim
Copy link
Collaborator

@compulim compulim commented Apr 26, 2023

Background

This pull request is to improve the DLASE chat adapter quality.

There are several issues found when we are pushing the quality bar.

Due to resources constraints, some tests are not done yet. The TODO.md list tests that need to be written, and areas that need to look at to make sure they are properly covered.

Specific changes

  • Bump to jest@29.5.0 and jest-environment-jsdom@29.5.0
    • Added Jest environment to every test files
    • Removed setupCrypto as it is no longer needed in latest jsdom
    • Removed config file for babel-jest as it is no longer required
  • Added API docs
  • Added scenario tests for DLASE
    • Using express and http-proxy-middleware to create a bot proxy
    • We can control this proxy to mimic disconnections or modify traffics
  • Bumped botframework-streaming for a development build
    • We will hold onto this development build until we release
  • Fix: when end() is called, it should end all operations and no further reconnect/revival is allowed
    • activity$ should be completed
    • connectionStatus$ should become Ended and completed
    • Call to reconnect() should throw immediately
    • Error should be observed when calling postActivity()
  • Fix: when postActivity() succeeded, it should next(activityId) followed by complete()
  • Fix: when postActivity() failed, it should error()
  • Fix: when disconnection is detected, it should reconnect
    • Should emit Connecting or FailedToConnect immediately
    • Reconnect should be delayed for 3-15 seconds, unless the previous connection was marked as stable
    • If the connection is established for more than a minute, it will be marked as stable
      • Stable connection will reset retry count back to 3
      • If a stable connection is disconnected, next reconnect will be done immediately
  • Fix: when reconnect() is called, it will kick off connection if it is not already connecting
    • If it is already connecting or connected, reconnect() will be no-op
    • If the connection is already ended, it will throw
  • Fix: when all connection retries exhausted, the connectionStatus$ should become FailedToConnect
    • After FailedToConnect is observed, call to reconnect() will reset the retry counter and reconnect
  • Fix: when retrying, it should signal Connecting once, despite it retry multiple times

@compulim compulim requested a review from a team as a code owner April 26, 2023 08:08
@compulim compulim changed the title Fix Direct Line ASE (Streaming) Fix reconnection scenarios and various behaviors of Direct Line ASE Apr 26, 2023
@compulim compulim merged commit 31ba05b into master Apr 26, 2023
@compulim compulim deleted the feat-dlase-e2e-test branch April 26, 2023 17:31
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