Skip to content

Commit

Permalink
work around context loss with async-listener
Browse files Browse the repository at this point in the history
[async-listener][1] is important module for APM tools and for long
stack traces. Promises make the concept of a long-stack-trace ambiguous
– as you can conceive the `then` callback as a continuation of either
the resolution context or the context that queued the `then`
callback ([more details][2]).

async-listener defaults to the resolution context. This is the wrong
default for how we are using promises here, resuling in APM tools
like Stackdriver Trace losing context.

We work-around the problem by not using the promise after it has
resolved.

[1]: https://www.npmjs.com/package/async-listener
[2]: othiym23/node-continuation-local-storage#64
  • Loading branch information
ofrobots committed Jun 21, 2017
1 parent bbece3c commit 3087093
Showing 1 changed file with 20 additions and 0 deletions.
20 changes: 20 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,26 @@ class Auth {
}

getAuthClient (callback) {
if (this.authClient) {
// This code works around an issue with context loss with async-listener.
// Strictly speaking, this should not be necessary as the call to
// authClientPromise.then(..) below would resolve to the same value.
// However, async-listener defaults to resuming the `then` callbacks with
// the context at the point of resolution rather than the context from the
// point where the `then` callback was added. In this case, the promise
// will be resolved on the very first incoming http request, and that
// context will become sticky (will be restored by async-listener) around
// the `then` callbacks for all subsequent requests.
//
// This breaks APM tools like Stackdriver Trace & others and tools like
// long stack traces (they will provide an incorrect stack trace).
//
// NOTE: this doesn't solve the problem generally. Any request concurrent
// to the first call to this function, before the promise resolves, will
// still lose context. We don't have a better solution at the moment :(.
return setImmediate(callback.bind(null, null, this.authClient));
}

var createAuthClientPromise = (resolve, reject) => {
var googleAuth = new GoogleAuth();

Expand Down

0 comments on commit 3087093

Please sign in to comment.