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 for #2776: 'null' response to Invoke when using AspNetCore integration package #2792

Merged
merged 1 commit into from
Oct 22, 2019

Conversation

beauzeaux
Copy link
Contributor

Fixed issue where the body of invokes with no response was 'null' rather than an empty body. This was causing all the TeamsActivityHandler delegates that didn't have a payload response to an invoke to fail in-flight on returning to Teams as one of the intermediaries attempts to parse the body as JSON if present.

@beauzeaux beauzeaux changed the title Fixed bug with 'null' response to Invoke Fix for #2776: 'null' response to Invoke when using AspNetCore integration package Oct 21, 2019
@coveralls
Copy link
Collaborator

coveralls commented Oct 21, 2019

Pull Request Test Coverage Report for Build 84685

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-1.4%) to 76.014%

Totals Coverage Status
Change from base Build 84533: -1.4%
Covered Lines: 7926
Relevant Lines: 10427

💛 - Coveralls

@johnataylor
Copy link
Member

Looks like a bug, and this fix looks good, we should take this fix.

However, one note this is in the old style ASP integration layer. The new style (i.e. using the BotFrameworkHttpAdapter class) provides a more natural and more flexible integration with ASP that sits much better with the dotnet core dependency injection.

@beauzeaux
Copy link
Contributor Author

beauzeaux commented Oct 21, 2019

I'll go ahead and merge then.

However, one note this is in the old style ASP integration layer. The new style (i.e. using the BotFrameworkHttpAdapter class) provides a more natural and more flexible integration with ASP that sits much better with the dotnet core dependency injection

I'm curious why this is the case, but we can discuss in a more appropriate channel than a PR thread.

Edit: I'm a fool - not used to this when I'm not the maintainer of the repository. Apologies for the close / reopen.

@beauzeaux beauzeaux closed this Oct 21, 2019
@beauzeaux beauzeaux reopened this Oct 21, 2019
@johnataylor johnataylor merged commit 2ae9937 into microsoft:master Oct 22, 2019
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.

None yet

3 participants