search

microsoft / BotBuilder-Samples

Welcome to the Bot Framework samples repository. Here you will find task-focused samples in C#, JavaScript/TypeScript, and Python to help you get started with the Bot Framework SDK!
https://github.com/Microsoft/botframework
MIT License
4.36k stars 4.87k forks source link

Ensure the SDK doesn't emit unhandled rejections #3148

Open stevengum opened 3 years ago

stevengum commented 3 years ago

Official Node 15 blog post

Snippet from blog post:

Throw on unhandled rejections

As of Node.js 15, the default mode for unhandledRejection is changed to throw (from warn). In throw mode, if an unhandledRejection hook is not set, the unhandledRejection is raised as an uncaught exception. Users that have an unhandledRejection hook should see no change in behavior, and it’s still possible to switch modes using the --unhandled-rejections=mode process flag.

Node.js has emitted an UnhandledPromiseRejectionWarning by default for many releases and the decision to switch the mode to throw was agreed to based on input from the results of the Node.js User Insights: Unhandled Promise Rejections survey, and finally a Node.js Technical Steering Committee vote. A special thank you to Mary Marchini for driving this decision forwards.

While the SDK doesn't officially support odd number versions of Node (aka non-LTS), addressing unhandled rejections will:

  1. Improve code quality of the SDK
  2. Prepare the SDK for Node.js 16
joshgummersall commented 3 years ago

Things to consider: updating eslint rules, listening for process.on('unhandledRejection') in the tests, and failing if emitted.

guy-microsoft commented 3 years ago

I encountered unhandled rejections when the callbackUrl of a request to a bot included a URL that could not be resolved (that resulted in getaddrinfo ENOTFOUND). I tried to reproduce it so I could provide more details but no luck. I will update here if I manage to reproduce it.

The error was: new RestError (/app/node_modules/botframework-connector/node_modules/@azure/ms-rest-js/dist/msRest.node.js

stevengum commented 3 years ago

We should rewrite the existing JS samples to have proper rejection handling. Perhaps follow the pattern set via https://github.com/microsoft/botbuilder-js/pull/3492