MicrosoftDocs / msteams-docs

Source for the Microsoft Teams developer platform documentation.
https://aka.ms/teamsdev
Creative Commons Attribution 4.0 International
286 stars 508 forks source link

Include code examples of what to do/what not to do when handling errors #10383

Open aosolis opened 8 months ago

aosolis commented 8 months ago

Guidance here needs to be much stronger: https://learn.microsoft.com/en-us/microsoftteams/platform/bots/how-to/conversations/subscribe-to-conversation-events?tabs=dotnet#handling-errors-in-conversation-events

Provide a sample of incorrect vs correct error handling code Give examples of cases that lead to nonsensical messages like the one in the screenshot. Tell developers how they can test these cases. These error conditions are often edge cases that developers might not know how to test until they see them in production after deployment. For example, the conversationUpdate that they receive when a user leaves the tenant.


Document Details

Do not edit this section. It is required for learn.microsoft.com ➟ GitHub issue linking.

microsoft-github-policy-service[bot] commented 8 months ago

Hi aosolis! Thank you for bringing this issue to our attention. We will investigate and if we require further information we will reach out in one business day. Please use this link to escalate if you don't get replies.

Best regards, Teams Platform

Vikram-MSFT commented 8 months ago

Hello @aosolis - Thanks for raising your feedbacks. We will look into it and let you know the updates.

Vikram-MSFT commented 8 months ago

Hello @aosolis **Provide a sample of incorrect vs correct error handling code:

Incorrect sample code C#**

public class AdapterWithErrorHandler : CloudAdapter { public AdapterWithErrorHandler(BotFrameworkAuthentication auth, ILogger logger, ConversationState conversationState = default) : base(auth, logger) { OnTurnError = async (turnContext, exception) => { // Log any leaked exception from the application. logger.LogError(exception, $"[OnTurnError] unhandled error : {exception.Message}");

            await turnContext.SendActivityAsync($"Sorry, it looks like something went wrong. Exception Caught: {exception.Message}");

            // Send a trace activity, which will be displayed in the Bot Framework Emulator
            await turnContext.TraceActivityAsync("OnTurnError Trace", exception.Message, "https://www.botframework.com/schemas/error", "TurnError");
        };
    }
}

**Node js**
adapter.onTurnError = async (context, error) => {
// This check writes out errors to console log .vs. app insights.
// NOTE: In production environment, you should consider logging this to Azure
// application insights.
console.error(`\n [onTurnError] unhandled error: ${ error }`);

// Send a trace activity, which will be displayed in Bot Framework Emulator
await context.sendTraceActivity(
    'OnTurnError Trace',
    `${ error }`,
    'https://www.botframework.com/schemas/error',
    'TurnError'
);

 // We should not send this kind of error to users.
 await context.sendActivity(`Sorry, it looks like something went wrong. Exception Caught: ${error}`);    

}; Correct Sample Code C# public class AdapterWithErrorHandler : CloudAdapter { public AdapterWithErrorHandler(BotFrameworkAuthentication auth, ILogger logger, ConversationState conversationState = default) : base(auth, logger) { OnTurnError = async (turnContext, exception) => { // Log any leaked exception from the application. logger.LogError(exception, $"[OnTurnError] unhandled error : {exception.Message}");

            // Uncomment below commented line for local debugging.
                            // During deployment, we should comment this line.  
            // await turnContext.SendActivityAsync($"Sorry, it looks like something went wrong. Exception Caught: {exception.Message}");

            // Send a trace activity, which will be displayed in the Bot Framework Emulator
            await turnContext.TraceActivityAsync("OnTurnError Trace", exception.Message, "https://www.botframework.com/schemas/error", "TurnError");
        };
    }
}

**node js:**
adapter.onTurnError = async (context, error) => {
// This check writes out errors to console log .vs. app insights.
// NOTE: In production environment, you should consider logging this to Azure
//       application insights.
console.error(`\n [onTurnError] unhandled error: ${ error }`);

// Send a trace activity, which will be displayed in Bot Framework Emulator
await context.sendTraceActivity(
    'OnTurnError Trace',
    `${ error }`,
    'https://www.botframework.com/schemas/error',
    'TurnError'
);

 // Uncomment below commented line for local debugging.
 // During production deployment we should comment this line.
 // await context.sendActivity(`Sorry, it looks like something went wrong. Exception Caught: ${error}`);    

};

Vikram-MSFT commented 8 months ago

Give examples of cases that lead to nonsensical messages like the one in the screenshot.

During error handling at global level, we should not send unnecessary message to users like below: adapter.onTurnError = async (context, error) => { await context.sendActivity(Sorry, it looks like something went wrong. Exception Caught: ${error});
};

Tell developers how they can test these cases. These error conditions are often edge cases that developers might not know how to test until they see them in production after deployment. For example, the conversationUpdate that they receive when a user leaves the tenant.

Developers can put a debugger and can test the different cases in the application errors in onTurnError method like below: adapter.onTurnError = async (context, error) => {

 // Comment this line during production deployment.
 await context.sendActivity(`Sorry, it looks like something went wrong. Exception Caught: ${error}`);    

};

ChetanSharma-msft commented 8 months ago

Hello @aosolis - Could you please review the above provided code and share your feedbacks.