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 context between two async requests #7

Merged
merged 4 commits into from
Nov 29, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
language: node_js
node_js:
- "4"
- "5"
- "8"
- "stable"

sudo: required
Expand Down
28 changes: 28 additions & 0 deletions lib/async-context.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
'use strict';

const asyncHooks = require('async_hooks');

class AsyncContext {
constructor() {
this.map = new Map();
asyncHooks.createHook({
init: (id, _type, triggerId) => {
if (this.map.has(triggerId))
this.map.set(id, this.map.get(triggerId));
},
destroy: (id) => this.map.delete(id)
}).enable();
}

get() {
const id = asyncHooks.executionAsyncId();
if (this.map.has(id))
return this.map.get(id);
}

set(val) {
this.map.set(asyncHooks.executionAsyncId(), val);
}
}

module.exports = new AsyncContext();
9 changes: 7 additions & 2 deletions lib/middlewares/express.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
'use strict';

const asyncContext = require('../async-context');

module.exports = {
buildMiddleware: function(provider) {
return function(req, res, next) {
provider.handler(req, res, next);
};
},
mainMiddleware: function(enable, authorize, handleRequest) {
mainMiddleware: function(enable, authorize, handleRequest, cls) {
return function(req, res, next) {
handleRequest(enable, authorize, req, res).then((handled) => {
res.locals.miniprofiler = req.miniprofiler;

asyncContext.set(req.miniprofiler);
Object.defineProperty(req, 'miniprofiler', { get: () => asyncContext.get() });

var render = res.render;
res.render = function() {
var renderArguments = arguments;
Expand All @@ -24,4 +29,4 @@ module.exports = {
}).catch(next);
};
}
};
};
6 changes: 5 additions & 1 deletion lib/middlewares/hapi.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

const asyncContext = require('../async-context');

module.exports = {
buildMiddleware: function(provider) {
var plugin = {
Expand All @@ -25,7 +27,9 @@ module.exports = {
register: (server, options, next) => {
server.ext('onRequest', function(request, reply) {
handleRequest(enable, authorize, request.raw.req, request.raw.res).then((handled) => {
request.app.miniprofiler = request.raw.req.miniprofiler;
asyncContext.set(request.raw.req.miniprofiler);
Object.defineProperty(request.app, 'miniprofiler', { get: () => asyncContext.get() });
Object.defineProperty(request.raw.req, 'miniprofiler', { get: () => asyncContext.get() });

if (!handled)
reply.continue();
Expand Down
7 changes: 6 additions & 1 deletion lib/middlewares/koa.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

const asyncContext = require('../async-context');

module.exports = {
buildMiddleware: function(provider) {
return function *(next) {
Expand All @@ -12,7 +14,10 @@ module.exports = {
mainMiddleware: function(enable, authorize, handleRequest) {
return function *(next) {
var handled = yield handleRequest(enable, authorize, this.req, this.res);
this.state.miniprofiler = this.req.miniprofiler;

asyncContext.set(this.req.miniprofiler);
Object.defineProperty(this.state, 'miniprofiler', { get: () => asyncContext.get() });
Object.defineProperty(this.req, 'miniprofiler', { get: () => asyncContext.get() });

if (this.render) {
var render = this.render;
Expand Down
14 changes: 6 additions & 8 deletions lib/miniprofiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,12 @@ function handleRequest(enable, authorize, req, res) {
}

if (!requestPath.startsWith(resourcePath)) {
var id = startProfiling(req, enabled, authorized);
var extension = startProfiling(req, enabled, authorized);
if (enabled) {
res.on('finish', () => {
stopProfiling(req);
stopProfiling(extension, req);
});
res.setHeader('X-MiniProfiler-Ids', `["${id}"]`);
res.setHeader('X-MiniProfiler-Ids', `["${extension.id}"]`);
}
return resolve(false);
}
Expand Down Expand Up @@ -279,21 +279,19 @@ function include(id) {
};

request.miniprofiler = currentRequestExtension;
return currentRequestExtension.id;

return currentRequestExtension;
}

/*
* Stops profiling the given request.
*/
function stopProfiling(request){
var extension = request.miniprofiler;
function stopProfiling(extension, request){
var time = process.hrtime();

extension.stopTime = time;
extension.stepGraph.stopTime = time;

delete request.miniprofiler;

var json = describePerformance(extension, request);
storage.set(extension.id, JSON.stringify(json));
}
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "miniprofiler",
"version": "1.2.3",
"version": "2.0.0",
"description": "A simple but effective mini-profiler.",
"main": "lib/miniprofiler.js",
"scripts": {
Expand Down Expand Up @@ -46,6 +46,7 @@
"koa": "^1.2.1",
"koa-route": "^2.4.2",
"koa-views": "^4.1.0",
"debug": "^2.6.1",
"mocha": "^2.5.3",
"pug": "^2.0.0-beta2",
"redis": "^2.6.1",
Expand Down
37 changes: 37 additions & 0 deletions tests/concurrent-async-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
'use strict';

var expect = require('chai').expect;

module.exports = function(server) {
describe('Concurrent Async Requests', function() {
before(server.setUp.bind(null, 'async'));
after(server.tearDown);

it('Each profile runs on its own context', function(done) {
let countDone = 0;
const partialDone = () => { if (++countDone === 2) done(); };

server.get('/', (err, response) => {
var ids = JSON.parse(response.headers['x-miniprofiler-ids']);
expect(ids).to.have.lengthOf(1);

server.post('/mini-profiler-resources/results/', { id: ids[0], popup: 1 }, (err, response, body) => {
var result = JSON.parse(body);
expect(result.Root.CustomTimings.async).to.have.lengthOf(2);
partialDone();
});
});

server.get('/?once=true', (err, response) => {
var ids = JSON.parse(response.headers['x-miniprofiler-ids']);
expect(ids).to.have.lengthOf(1);

server.post('/mini-profiler-resources/results/', { id: ids[0], popup: 1 }, (err, response, body) => {
var result = JSON.parse(body);
expect(result.Root.CustomTimings.async).to.have.lengthOf(1);
partialDone();
});
});
});
});
};
21 changes: 21 additions & 0 deletions tests/servers/async-provider.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
'use strict';

module.exports = function(obj) {
return {
name: 'dummy-async',
handler: function(req, res, next) {
obj.asyncFn = function() {
const timing = req.miniprofiler.startTimeQuery('async', 'dummy call');

return new Promise(resolve => {
setTimeout(() => {
req.miniprofiler.stopTimeQuery(timing);
resolve();
}, 25);
});
};

next();
}
};
};
5 changes: 5 additions & 0 deletions tests/servers/dummy-module.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
'use strict';

module.exports = {
asyncFn: () => Promise.resolve()
};
19 changes: 19 additions & 0 deletions tests/servers/express/async.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
'use strict';

var miniprofiler = require('../../../lib/miniprofiler.js');
var dummyModule = require('../dummy-module');
var express = require('express');

var app = express();

app.use(miniprofiler.express());
app.use(miniprofiler.express.for(require('../async-provider.js')(dummyModule)));

app.get('/', (req, res) => {
dummyModule.asyncFn().then(() => {
Promise.resolve(req.query.once ? undefined : dummyModule.asyncFn())
.then(() => res.send(res.locals.miniprofiler.include()));
});
});

module.exports = app;
29 changes: 29 additions & 0 deletions tests/servers/hapi/async.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
'use strict';

var miniprofiler = require('../../../lib/miniprofiler.js');
var dummyModule = require('../dummy-module');
const Hapi = require('hapi');

const server = new Hapi.Server();
server.connection({ port: 8083 });

server.register(miniprofiler.hapi(), (err) => {
if (err) throw err;
});

server.register(miniprofiler.hapi.for(require('../async-provider.js')(dummyModule)), (err) => {
if (err) throw err;
});

server.route({
method: 'GET',
path:'/',
handler: function(request, reply) {
dummyModule.asyncFn().then(() => {
Promise.resolve(request.query.once ? undefined : dummyModule.asyncFn())
.then(() => reply(request.app.miniprofiler.include()));
});
}
});

module.exports = server;
19 changes: 19 additions & 0 deletions tests/servers/koa/async.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
'use strict';

var miniprofiler = require('../../../lib/miniprofiler.js');
var dummyModule = require('../dummy-module');
var koa = require('koa');
var route = require('koa-route');
var app = koa();

app.use(miniprofiler.koa());
app.use(miniprofiler.koa.for(require('../async-provider.js')(dummyModule)));

app.use(route.get('/', function *(){
yield dummyModule.asyncFn().then(() => {
return Promise.resolve(this.query.once ? undefined : dummyModule.asyncFn())
.then(() => { this.body = this.state.miniprofiler.include(); });
});
}));

module.exports = app;