Skip to content

Commit 8841f83

Browse files
author
Danilo Resende
committed
Handle context between two async requests
When using `req.miniprofiler` to track the miniprofiler extension, two requests can dispute this resource where the reference will always point to the last request. So a scenario with async timings, the first request can endup using `req.miniprofiler` that points to the second request extension. (Check the tests to see a scenario simulation) Using NodeJS "async_hooks" we can track the reference to the correct extension for each request. Related to: MiniProfiler#4
1 parent 426fb18 commit 8841f83

File tree

8 files changed

+141
-9
lines changed

8 files changed

+141
-9
lines changed

lib/miniprofiler.js

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
var url = require('url');
1616
var ui = require('./ui.js');
1717
var clientParser = require('./client-parser.js');
18+
var ContinuationLocalStorage = require('asyncctx').ContinuationLocalStorage;
1819

1920
const hostname = require('os').hostname;
2021
var ignoredPaths = [];
@@ -27,6 +28,7 @@ var ignoredPaths = [];
2728
RedisStorage: require('./storages/redis.js')
2829
};
2930

31+
var cls = new ContinuationLocalStorage();
3032
var storage = new exports.storage.InMemoryStorage({ max: 100, maxAge: 1000 * 60 * 60 });
3133

3234
exports.configure = configure;
@@ -74,12 +76,12 @@ function handleRequest(enable, authorize, req, res) {
7476
}
7577

7678
if (!requestPath.startsWith(resourcePath)) {
77-
var id = startProfiling(req, enabled, authorized);
79+
var extension = startProfiling(req, enabled, authorized);
7880
if (enabled) {
7981
res.on('finish', () => {
80-
stopProfiling(req);
82+
stopProfiling(extension, req);
8183
});
82-
res.setHeader('X-MiniProfiler-Ids', `["${id}"]`);
84+
res.setHeader('X-MiniProfiler-Ids', `["${extension.id}"]`);
8385
}
8486
return resolve(false);
8587
}
@@ -278,22 +280,21 @@ function include(id) {
278280
return enabled && authorized ? include(currentRequestExtension.id) : '';
279281
};
280282

281-
request.miniprofiler = currentRequestExtension;
282-
return currentRequestExtension.id;
283+
cls.setContext(currentRequestExtension);
284+
Object.defineProperty(request, 'miniprofiler', { get: () => cls.getContext() });
285+
286+
return currentRequestExtension;
283287
}
284288

285289
/*
286290
* Stops profiling the given request.
287291
*/
288-
function stopProfiling(request){
289-
var extension = request.miniprofiler;
292+
function stopProfiling(extension, request){
290293
var time = process.hrtime();
291294

292295
extension.stopTime = time;
293296
extension.stepGraph.stopTime = time;
294297

295-
delete request.miniprofiler;
296-
297298
var json = describePerformance(extension, request);
298299
storage.set(extension.id, JSON.stringify(json));
299300
}

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
"license": "Apache-2.0",
2727
"readmeFilename": "README.md",
2828
"dependencies": {
29+
"asyncctx": "^1.0.9",
2930
"lru-cache": "^4.0.1"
3031
},
3132
"tags": [

tests/concurrent-async-test.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
'use strict';
2+
3+
var expect = require('chai').expect;
4+
5+
module.exports = function(server) {
6+
describe('Concurrent Async Requests', function() {
7+
before(server.setUp.bind(null, 'async'));
8+
after(server.tearDown);
9+
10+
it('Each profile runs on its own context', function(done) {
11+
let countDone = 0;
12+
const partialDone = () => { if (++countDone === 2) done(); };
13+
14+
server.get('/', (err, response) => {
15+
var ids = JSON.parse(response.headers['x-miniprofiler-ids']);
16+
expect(ids).to.have.lengthOf(1);
17+
18+
server.post('/mini-profiler-resources/results/', { id: ids[0], popup: 1 }, (err, response, body) => {
19+
var result = JSON.parse(body);
20+
expect(result.Root.CustomTimings.async).to.have.lengthOf(2);
21+
partialDone();
22+
});
23+
});
24+
25+
server.get('/?once=true', (err, response) => {
26+
var ids = JSON.parse(response.headers['x-miniprofiler-ids']);
27+
expect(ids).to.have.lengthOf(1);
28+
29+
server.post('/mini-profiler-resources/results/', { id: ids[0], popup: 1 }, (err, response, body) => {
30+
var result = JSON.parse(body);
31+
expect(result.Root.CustomTimings.async).to.have.lengthOf(1);
32+
partialDone();
33+
});
34+
});
35+
});
36+
});
37+
};

tests/servers/async-provider.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
'use strict';
2+
3+
module.exports = function(obj) {
4+
return {
5+
name: 'dummy-async',
6+
handler: function(req, res, next) {
7+
obj.asyncFn = function() {
8+
const timing = req.miniprofiler.startTimeQuery('async', 'dummy call');
9+
10+
return new Promise(resolve => {
11+
setTimeout(() => {
12+
req.miniprofiler.stopTimeQuery(timing);
13+
resolve();
14+
}, 25);
15+
});
16+
};
17+
18+
next();
19+
}
20+
};
21+
};

tests/servers/dummy-module.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
'use strict';
2+
3+
module.exports = {
4+
asyncFn: () => Promise.resolve()
5+
};

tests/servers/express/async.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
'use strict';
2+
3+
var miniprofiler = require('../../../lib/miniprofiler.js');
4+
var dummyModule = require('../dummy-module');
5+
var express = require('express');
6+
7+
var app = express();
8+
9+
app.use(miniprofiler.express());
10+
app.use(miniprofiler.express.for(require('../async-provider.js')(dummyModule)));
11+
12+
app.get('/', (req, res) => {
13+
dummyModule.asyncFn().then(() => {
14+
Promise.resolve(req.query.once ? undefined : dummyModule.asyncFn())
15+
.then(() => res.send(res.locals.miniprofiler.include()));
16+
});
17+
});
18+
19+
module.exports = app;

tests/servers/hapi/async.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
'use strict';
2+
3+
var miniprofiler = require('../../../lib/miniprofiler.js');
4+
var dummyModule = require('../dummy-module');
5+
const Hapi = require('hapi');
6+
7+
const server = new Hapi.Server();
8+
server.connection({ port: 8083 });
9+
10+
server.register(miniprofiler.hapi(), (err) => {
11+
if (err) throw err;
12+
});
13+
14+
server.register(miniprofiler.hapi.for(require('../async-provider.js')(dummyModule)), (err) => {
15+
if (err) throw err;
16+
});
17+
18+
server.route({
19+
method: 'GET',
20+
path:'/',
21+
handler: function(request, reply) {
22+
dummyModule.asyncFn().then(() => {
23+
Promise.resolve(request.query.once ? undefined : dummyModule.asyncFn())
24+
.then(() => reply(request.app.miniprofiler.include()));
25+
});
26+
}
27+
});
28+
29+
module.exports = server;

tests/servers/koa/async.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
'use strict';
2+
3+
var miniprofiler = require('../../../lib/miniprofiler.js');
4+
var dummyModule = require('../dummy-module');
5+
var koa = require('koa');
6+
var route = require('koa-route');
7+
var app = koa();
8+
9+
app.use(miniprofiler.koa());
10+
app.use(miniprofiler.koa.for(require('../async-provider.js')(dummyModule)));
11+
12+
app.use(route.get('/', function *(){
13+
yield dummyModule.asyncFn().then(() => {
14+
return Promise.resolve(this.query.once ? undefined : dummyModule.asyncFn())
15+
.then(() => { this.body = this.state.miniprofiler.include(); });
16+
});
17+
}));
18+
19+
module.exports = app;

0 commit comments

Comments
 (0)