twilio-labs / runtime-helpers Goto Github PK

Problem

Today a customer might be writing some unintentionally "leaky" code where they call callback(...) before all asynchronous operations they started have completed. This could be because they ran a method that returns a Promise and they haven't awaited it or they ran a function that receives it's own callback method and they haven't adjusted their code accordingly.

As a result this can create unintended behavior in a variety of ways. Some of these operations might only complete on subsequent executions of the Function and in other cases the result might not be received or the logging will be off.

A very basic example for testing:

exports.handler = function (context, event, callback) {
  console.log('1')
  setTimeout(() => {
    console.log('3');
  }, 1000);
  console.log('2');
  callback(null, {});
};

A customer should have to refactor this code by moving the callback(null, {}) into the setTimeout after console.log('3') in order to make sure all things get appropriately logged.

Proposed Solution

Create a higher order function that can be used to be warned about any unfinished asynchronous code in the debugger:

const { warnOnUnfinishedCode } = require('@twilio-labs/runtime-helpers');

exports.handler = warnOnUnfinishedCode(function (context, event, callback) {
  console.log('1')
  setTimeout(() => {
    console.log('3');
  }, 1000);
  console.log('2');
  callback(null, {});
});

In this case the library would log a warning to the debugger (using console.warn) to inform the customer that on line 5 they created a timeout that has not yet completed and that the callback(null, {}) needs to be moved into it.

Once we've proven the usefulness of this and it's stability we could decide to directly offer this as part of the Runtime Handler for all customers. In the meantime this should help customers (and support) to debug those issues.

Technical details

In order to detect those open handles we need to use async_hooks (see documentation) to track init and destroy events, filter out any that are not relevant to customers (either because they are Node.js or Functions internal) and at the end of the callback execution trigger a warning if any async events were created in init but never destroyed in destroy.

Each event in the init should come with a type for better filtering and we should be able to use Error stacks (i.e. create an error object and inspect the error.stack) to determine the line that the async handle was created on.