Botbuilder-js: Missing handlers throwing 501s result in unhandled promise messages

[christopheranderson]

Versions

What package version of the SDK are you using: 4.6.0-preview1
What nodejs version are you using: v10 and v12
What browser version are you using: n/a
What os are you using: windows 10

Describe the bug

Give a clear and concise description of what the bug is.

There is an error being thrown from processActivity which isn't being handled in a many places in the SDK, leading to errors written to stderr (and could crash the process depending on the strict mode).

To Reproduce

Steps to reproduce the behavior:

1. Try this sample: https://github.com/microsoft/BotBuilder-Samples/blob/master/experimental/teams/js/actionBasedMessagingExtension/src/actionBasedMessagingExtensionBot.ts
2. Try the create-preview action
3. Enter whatever for input
4. Click next
5. Click edit
   6.Error will be thrown at this point and visible in stderr

(node:20036) UnhandledPromiseRejectionWarning: Error: BotFrameworkAdapter.processActivity(): 501 ERROR
    at BotFrameworkAdapter.processActivity (C:\w\msft\fuse\bot\BotBuilder-Samples\experimental\teams\js\actionBasedMessagingExtension\node_modules\botbuilder\lib\botFrameworkAdapter.js:589:23)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
(node:20036) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)
(node:20036) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Expected behavior

Give a clear and concise description of what you expected to happen.

I don't think processActivity needs to throw exceptions (it should pass them to the turnContext and potentially log them to a logger).

Screenshots

If applicable, add screenshots to help explain your problem.

n/a

Additional context

Add any other context about the problem here.

I have talked to Steven about a possible fix for this already that involves using an anonymous async lambda with a .catch sink to a logger of some kind as well as the other stuff covered in expected behavior.

[bug]

[kfolkes]

@stevengum Is there an ETA on this being fixed? I too and facing similar issue.

[stevengum]

@kfolkes, the team is currently talking it over for our R8 sprint. In addition to the unhandled promise errors handling, I'd like to very deliberately add a logger concept into the SDK and to use the logger here in these unhandled promise errors.

[johnataylor]

I've been investigating this and it looks like this issue was actually fixed before our release. Note this was filed on a prerelease.

Also note the samples are OK because there is a wide exception catch in the adapter.processActivity

[johnataylor]

closing for R8

[agladboy]

@johnataylor We are still hitting the same issue time to time, would you please share when will this fix be available formally?

[cindylarison]

I am seeing a lot of these in my logs. This is with "botbuilder": "~4.8.0",