Skip to content

Commit

Permalink
Check if properRoute is found before acting on it (#73)
Browse files Browse the repository at this point in the history
* Check if properRoute is found before acting on it

In the cases where the request route is not matched to any Koa route,
bail out before trying to format the path string.

Issue: #59

* Add test for koa middleware unmatched route scenario

Issue: #59

* Add integration test for wildcard path scenario

Issue: #59
  • Loading branch information
kdhira committed Mar 8, 2022
1 parent b9ab859 commit d11c383
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/koa-middleware.js
Expand Up @@ -33,7 +33,7 @@ class KoaMiddleware {
}
}

_handleResponse (ctx) {
_handleResponse(ctx) {
const responseLength = parseInt(ctx.response.get('Content-Length')) || 0;

const route = this._getRoute(ctx) || 'N/A';
Expand Down Expand Up @@ -93,6 +93,12 @@ class KoaMiddleware {
}
});

// If proper route is not found, send an undefined route
// The caller is responsible for setting a default "N/A" route in this case
if (!properRoute) {
return undefined;
}

let route = properRoute.path;
route = route.endsWith('/') ? route.substring(0, route.length - 1) : route;
return route;
Expand Down
17 changes: 17 additions & 0 deletions test/integration-tests/koa/middleware-test.js
Expand Up @@ -679,6 +679,23 @@ describe('when using koa framework', () => {
});
});
});
describe('when calling wildcard endpoint', function () {
before(() => {
const wildcardPath = '/wild-path/' + Math.floor(Math.random() * 10);
return supertest(app)
.get(wildcardPath)
.expect(200)
.then((res) => {});
});
it('should add it to the histogram', () => {
return supertest(app)
.get('/metrics')
.expect(200)
.then((res) => {
expect(res.text).to.contain('method="GET",route="N/A",code="200"');
});
});
});
it('should get metrics as json', () => {
return supertest(app)
.get('/metrics.json')
Expand Down
6 changes: 6 additions & 0 deletions test/integration-tests/koa/server/koa-server.js
Expand Up @@ -78,4 +78,10 @@ router.post('/test', async (ctx, next) => {
return next();
});

router.get('/wild-path/(.*)', (ctx, next) => {
ctx.status = 200;
ctx.body = { message: 'Wildcard route reached!' };
return next();
});

module.exports = app;
51 changes: 51 additions & 0 deletions test/unit-test/metric-middleware-koa-test.js
Expand Up @@ -653,6 +653,57 @@ describe('metrics-middleware', () => {
Prometheus.register.clear();
});
});
describe('when using middleware request and path not matched to any specific route', () => {
let match, func, req, res, ctx, next, requestSizeObserve, requestTimeObserve, endTimerStub;
before(async () => {
match = sinon.stub();
match.onFirstCall().returns({ path: [] });
match.onSecondCall().returns({ path: [{ path: '/(.*)' }] });
next = sinon.stub();
req = httpMocks.createRequest({
url: '/not-path',
method: 'GET',
body: {
foo: 'bar'
},
headers: {
'content-length': '25'
}
});
req.socket = {};
res = httpMocks.createResponse({
eventEmitter: EventEmitter
});
res.statusCode = 404;
ctx = { req: req, res: res, request: req, response: res, router: { match: match }, _matchedRoute: '/(.*)', originalUrl: '/not-path' };
func = await middleware();
endTimerStub = sinon.stub();
requestTimeObserve = sinon.stub(Prometheus.register.getSingleMetric('http_request_duration_seconds'), 'startTimer').returns(endTimerStub);
func(ctx, next);
requestSizeObserve = sinon.spy(Prometheus.register.getSingleMetric('http_request_size_bytes'), 'observe');
res.emit('finish');
});
it('should update the histogram with the elapsed time and size', () => {
sinon.assert.calledWithExactly(requestSizeObserve, {
method: 'GET',
route: 'N/A',
code: 404
}, 25);
sinon.assert.called(requestTimeObserve);
sinon.assert.calledWith(endTimerStub, {
method: 'GET',
route: 'N/A',
code: 404
});
sinon.assert.calledOnce(requestTimeObserve);
sinon.assert.calledOnce(endTimerStub);
});
after(() => {
requestSizeObserve.restore();
requestTimeObserve.restore();
Prometheus.register.clear();
});
});
describe('when _getConnections called', () => {
let Middleware, server, numberOfConnectionsGauge, koaMiddleware, prometheusStub;
before(() => {
Expand Down

0 comments on commit d11c383

Please sign in to comment.