Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: avoid fake .then() method on mysql2 Query class #501

Merged
merged 2 commits into from
May 9, 2022

Conversation

mickdekkers
Copy link
Contributor

@mickdekkers mickdekkers commented Apr 26, 2022

Issue #, if available: #452

Description of changes:

The mysql2 package has a Query class with a .then() method which throws an error when called.
This confuses the current patching code, which sees this object as a Promise and calls .then() on it.

As a result, code like this currently breaks with an error:

const captureMySQL = require('aws-xray-sdk-mysql');
const {createConnection} = captureMySQL(require('mysql2'));

const connection = createConnection(someConfig);
const queryStream = connection
        .query(someSql) // <-- returns Query class, error is thrown here
        .stream({highWaterMark: 10});

// (Attempt to) do stuff with queryStream
queryStream.pipe(someFileOnDisk);

This PR fixes this unfortunate interaction by performing an additional check to determine whether a returned value is actually a Promise.

Beyond the new unit test (which fails without the change and passes with it), I can confirm this change has fixed the error in our application.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Fixes: #452

Copy link
Contributor

@willarmiros willarmiros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this fix!

@willarmiros willarmiros merged commit 0124e92 into aws:master May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MySQL capture failing for mysql2 pool queries with promises
2 participants