Skip to content

Commit c1893a7

Browse files
committed
work around context loss with async-listener
async-listener 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
1 parent bbece3c commit c1893a7

File tree

1 file changed

+20
-0
lines changed

1 file changed

+20
-0
lines changed

index.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,26 @@ class Auth {
3333
}
3434

3535
getAuthClient (callback) {
36+
if (this.authClient) {
37+
// This code works around an issue with context loss with async-listener.
38+
// Strictly speaking, this should not be necessary as the call to
39+
// authClientPromise.then(..) below would resolve to the same value.
40+
// However, async-listener defaults to resuming the `then` callbacks with
41+
// the context at the point of resolution rather than the context from the
42+
// point where the `then` callback was added. In this case, the promise
43+
// will be resolved on the very first incoming http request, and that
44+
// context will become sticky (will be restored by async-listener) around
45+
// the `then` callbacks for all subsequent requests.
46+
//
47+
// This breaks APM tools like Stackdriver Trace & others and tools like
48+
// long stack traces (they will provide an incorrect stack trace).
49+
//
50+
// NOTE: this doesn't solve the problem generally. Any request concurrent
51+
// to the first call to this function, before the promise resolves, will
52+
// still lose context. We don't have a better solution at the moment :(.
53+
return setImmediate(callback.bind(null, null, this.authClient));
54+
}
55+
3656
var createAuthClientPromise = (resolve, reject) => {
3757
var googleAuth = new GoogleAuth();
3858

0 commit comments

Comments
 (0)