microsoft / botbuilder-dotnet

Welcome to the Bot Framework SDK for .NET repository, which is the home for the libraries and packages that enable developers to build sophisticated bot applications using .NET.
https://github.com/Microsoft/botframework
MIT License
864 stars 480 forks source link

The ShowTypingMiddleware throws System.ObjectDisposedException when an exception occurs in the bot #6752

Closed rliberoff closed 2 months ago

rliberoff commented 4 months ago

Version

I'm using 4.22.2

Describe the bug

When using the ShowTypingMiddleware in a CloudAdapter, it thows an System.ObjectDisposedException with message Cannot access a disposed object. Object name: 'Get'. after an exception has been handled by the OnTurnError error handler.

This is the stack trace:

at Microsoft.Bot.Builder.TurnContextStateCollection.Get[T](String key)
at Microsoft.Bot.Builder.TurnContextStateCollection.Get[T]()
at Microsoft.Bot.Builder.CloudAdapterBase.<SendActivitiesAsync>d__9.MoveNext()
at Microsoft.Bot.Builder.ShowTypingMiddleware.<SendTypingActivityAsync>d__7.MoveNext()
at Microsoft.Bot.Builder.ShowTypingMiddleware.<SendTypingAsync>d__6.MoveNext()
at Microsoft.Bot.Builder.ShowTypingMiddleware.<FinishTypingTaskAsync>d__9.MoveNext()
at Microsoft.Bot.Builder.ShowTypingMiddleware.<ProcessTypingAsync>d__10.MoveNext()
at Microsoft.Bot.Builder.ShowTypingMiddleware.<OnTurnAsync>d__4.MoveNext()
at Microsoft.Bot.Builder.MiddlewareSet.<ReceiveActivityWithStatusAsync>d__3.MoveNext()
at Microsoft.Bot.Builder.BotAdapter.<RunPipelineAsync>d__23.MoveNext()" | string

To Reproduce

Just create the simplest bot possible and Use the ShowTypingMiddleware. Produce an exception to be capture and handled by the CloudAdapter with the OnTurnError handler. Then throw a second exception and you will get this error.

Expected behavior

This middleware should show a typing indicator in Microsoft Teams and also in a web chat without throwing exceptions.

Screenshots

N/A

Additional context

N/A

rliberoff commented 4 months ago

Maybe the question is – if there is no bug – how to correctly handle exceptions in the CloudAdapter when using the ShowTypingMiddleware.

Please advice.

Thank you.

rliberoff commented 4 months ago

Menawhile we created a custom middleware. It is actually a copy of yours, but with a slightly different FinishTypingTaskAsync method, which is where we found the issue occurs.

    private async Task FinishTypingTaskAsync(ITurnContext turnContext)
    {
        if (string.IsNullOrEmpty(turnContext?.Activity?.Conversation?.Id) && !tasks.ContainsKey(turnContext.Activity.Conversation.Id))
        {
            return;
        }

        // Cancel the typing loop.
        tasks.TryGetValue(turnContext.Activity.Conversation.Id, out var item);
        var (typingTask, cts) = item;
        cts?.Cancel();
        cts?.Dispose();
        if (typingTask != null)
        {
            if (typingTask.Exception == null)
            {
                await typingTask.ConfigureAwait(false);
            }

            typingTask.Dispose();
        }

        tasks.TryRemove(turnContext.Activity.Conversation.Id, out _);
    }

Please notice the if (typingTask.Exception == null) which detects if there is an exception in the (previous) task.

Thank you.

JhontSouth commented 3 months ago

Hi @rliberoff, We were able to reproduce the issue by forcing an error in the OnMessageActivityAsync method and another one in the OnTurnError. We'll analyze how the correct behavior should be.

rliberoff commented 3 months ago

Hi @JhontSouth!

Thank you. If there’s anything more I can assist you with, please don’t hesitate to reach out.

tracyboehrer commented 2 months ago

Merged in https://github.com/microsoft/botbuilder-dotnet/pull/6772. Available next release.