[Mysql2] resolveArguments incorrectly assumes callback is present causing error

[bryanvaz]

Issue

The resolveArguments function to determine the arguments of a mysql connection.query or connection.execute incorrectly assumes the callback position for the mysql2 driver is the second argument.

The arguments of connection.query for the mysql2 driver are: (from mysql2/packages/mysql/lib/mysql_p.js#L146)

query(sql, values, cb) { ... }

By inspecting the contents of arguments in the XRay mysql wrapper when doing a complex INSERT (from aws-xray-sdk-node/packages/mysql/lib/mysql_p.js#L161) using the following code

function captureOperation(name) {
  return function() {
    var args = resolveArguments(arguments);
    console.log('arguments: ', arguments)
  ...
  }
}

... the results are:

contents of arguments

{
  '0': {
    sql: '\n' +
      '        INSERT INTO session (id, token_exp, known_ips)\n' +
      '          VALUES\n' +
      '              (?, ?, ? )\n' +
      '          ON DUPLICATE KEY UPDATE\n' +
      '              id = ?, token_exp = ?, known_ips = ?\n' +
      '      '
  },
  '1': [
    'aS9e9ng4AKqb6sKU3h_wN',
    1615328288,
    '[]',
    'aS9e9ng4AKqb6sKU3h_wN',
    1615328288,
    '[]'
  ],
  '2': [Function (anonymous)]
}

Note that the value of arguments[0] is an Object. This means that arguments test in resolveArguments flows onto the wrong branch when testing using if (argsObj[0] instanceof Object) { (from aws-xray-sdk-node/packages/mysql/lib/mysql_p.js#L143)

I only noticed this issue when using knex and async/await, however I assume other newer ORMs or query builders are using the mysql2 driver in the same way.

Suggested Fix

Account for this possibility with additional tests in resolveArguments as below:

function resolveArguments(argsObj) {
  var args = {};

  if (argsObj && argsObj.length > 0) {
    if (argsObj[0] instanceof Object) {
      args.sql = argsObj[0].sql;

      if (argsObj[0].timeout) {
        args.timeout = argsObj[0].timeout;
      }

      // New Code for suggested change
      if (argsObj[0].values) {
        args.values = argsObj[0].values;
      } else if(typeof argsObj[2] === 'function'){
        args.values = typeof argsObj[1] !== 'function' ? argsObj[1] : null;
      }
      args.callback = typeof argsObj[1] === 'function' ? argsObj[1] : (typeof argsObj[2] === 'function' ? argsObj[2] : undefined);
      
      // Old Code
      // args.values = argsObj[0].values;
      // args.callback = argsObj[1];
      if (!argsObj[1] && argsObj[0].on instanceof Function) args.sql = argsObj[0];
    } else {
      args.sql = argsObj[0];
      args.values = typeof argsObj[1] !== 'function' ? argsObj[1] : null;
      args.callback = typeof argsObj[1] === 'function' ? argsObj[1] : (typeof argsObj[2] === 'function' ? argsObj[2] : undefined);
    }

    args.segment = (argsObj[argsObj.length-1] != null && argsObj[argsObj.length-1].constructor && (argsObj[argsObj.length-1].constructor.name === 'Segment' ||
      argsObj[argsObj.length-1].constructor.name === 'Subsegment')) ? argsObj[argsObj.length-1] : null;
  }

  return args;
}

Implications for not implementing change

mysql2 based queries will fail on the following line because cb is not a function (from aws-xray-sdk-node/packages/mysql/lib/mysql_p.js#L188):

function captureOperation(name) {
  return function() {
    var args = resolveArguments(arguments);
    ...
    if (args.callback) {
      var cb = args.callback; // incorrectly set to arguments[1] which are `values`

      if (AWSXRay.isAutomaticMode()) {
        args.callback = function autoContext(err, data) {
          var session = AWSXRay.getNamespace();

          session.run(function() {
            AWSXRay.setSegment(subsegment);
            cb(err, data); // FAILS HERE because 'cb' is not a function (is the array fo values)
          });
          ...
        };
      }

Environment

Tested with:

• [email protected]
• [email protected]
• [email protected]