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: [#6683] Timeout issue when using DLASE #6696

Merged
merged 2 commits into from
Oct 6, 2023

Conversation

sw-joelmut
Copy link
Collaborator

@sw-joelmut sw-joelmut commented Oct 5, 2023

Fixes #6683

Description

This PR fixes an issue where, while using DLASE, the bot threw a timeout issue while streaming, due to not generating a response in the initial request.
Therefore, if the streaming process in the initial request takes longer than 15 seconds to finalize and generate a response, the bot will throw a timeout issue.
Sending a response (HTTP 202 (Accepted)) before starting to process all streaming requests addressed the issue and will not generate a timeout issue.

Specific Changes

  • Add SendResponseAsync call to the StreamingSession class to send an HTTP 202 (Accepted) response before starting processing all streaming information.
  • Add Accepted StatusCode condition to ProtocolAdapter for Streaming and StreamingSession for Connector.Streaming. This allows to detect when the Accepted response is taking place and can proceed to ignore the response and let the actual OK response to be used after streaming.

Testing

The following image shows the timeout issue before and after the fix.

Note: consistently was timing out around 25 to 35 activities, with the fix, it is no longer the case.

imagen

@sw-joelmut sw-joelmut added the Automation: No parity PR does not need to be applied to other languages. label Oct 5, 2023
@sw-joelmut sw-joelmut requested a review from a team as a code owner October 5, 2023 12:34
@tracyboehrer
Copy link
Member

@sw-joelmut It appears some Microsoft.Bot.Connector.Streaming.Tests are failing.

@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 371447

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 20 unchanged lines in 4 files lost coverage.
  • Overall coverage increased (+0.004%) to 73.455%

Files with Coverage Reduction New Missed Lines %
/libraries/AdaptiveExpressions/BuiltinFunctions/GetNextViableTime.cs 1 90.91%
/libraries/AdaptiveExpressions/BuiltinFunctions/GetPreviousViableTime.cs 1 90.91%
/libraries/Microsoft.Bot.Connector.Streaming/Application/TimerAwaitable.cs 1 68.25%
/libraries/Microsoft.Bot.Connector.Streaming/Session/StreamingSession.cs 17 91.2%
Totals Coverage Status
Change from base Build 369561: 0.004%
Covered Lines: 24147
Relevant Lines: 32873

💛 - Coveralls

@BruceHaley
Copy link
Contributor

✔️ No Binary Compatibility issues for Microsoft.Bot.Streaming.dll

@sw-joelmut
Copy link
Collaborator Author

@sw-joelmut It appears some Microsoft.Bot.Connector.Streaming.Tests are failing.

Changes applied, and updated the PR's description, thanks!

@tracyboehrer tracyboehrer merged commit 046725a into main Oct 6, 2023
14 of 15 checks passed
@tracyboehrer tracyboehrer deleted the southworks/fix/dlase-timeout branch October 6, 2023 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Automation: No parity PR does not need to be applied to other languages.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Timeout issue when using DLASE
4 participants