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

Enable calculating id based on http request #47

Closed
wants to merge 11 commits into from
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,9 @@ These are the available config options for the middleware/plugin functions. All
// Used if useHeader/echoHeader is set to true.
headerName: 'X-Request-Id',
// A custom function to generate your request ids (default: UUID v1).
// Framework-specific request object is passed as the argument, so it could be optionally used to calculate the id.
// Ignored if useHeader is set to true.
requestIdFactory: () => 'Your request id',
requestIdFactory: (req) => 'Your request id',
// Use request id generated by Fastify instead of generating a new id.
// Only available for the Fastify plugin.
useFastifyRequestId: false,
Expand Down
2 changes: 1 addition & 1 deletion index.d.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { IncomingMessage, ServerResponse } from 'http'

export type RequestIdFactory = () => unknown
export type RequestIdFactory = (req?: IncomingMessage|any) => unknown

export interface IOptions {
// Default: false
Expand Down
20 changes: 10 additions & 10 deletions src/rtracer.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,15 @@ const expressMiddleware = (setResHeaderFn) => {
return ({
useHeader = false,
headerName = 'X-Request-Id',
requestIdFactory = uuidv1,
requestIdFactory = null,
puzpuzpuz marked this conversation as resolved.
Show resolved Hide resolved
echoHeader = false
} = {}) => {
return (req, res, next) => {
let requestId
if (useHeader) {
requestId = req.headers[headerName.toLowerCase()]
}
requestId = requestId || requestIdFactory()
requestId = requestId || (requestIdFactory ? requestIdFactory(req) : uuidv1())

if (echoHeader) {
setResHeaderFn(res, headerName, requestId)
Expand Down Expand Up @@ -84,7 +84,7 @@ const fastifyPlugin = (fastify, options, next) => {
useHeader = false,
headerName = 'X-Request-Id',
useFastifyRequestId = false,
requestIdFactory = uuidv1,
requestIdFactory = null,
echoHeader = false
} = options

Expand All @@ -96,7 +96,7 @@ const fastifyPlugin = (fastify, options, next) => {
if (useFastifyRequestId) {
requestId = requestId || request.id
}
requestId = requestId || requestIdFactory()
requestId = requestId || (requestIdFactory ? requestIdFactory(request.raw) : uuidv1())
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
requestId = requestId || (requestIdFactory ? requestIdFactory(request.raw) : uuidv1())
requestId = requestId || (requestIdFactory ? requestIdFactory(request) : uuidv1())

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The raw contains all the info one might need from the request and will be compatible between all versions of fastify.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other non-Express middlewares and plugins from this PR provide framework specific request objects, so having request.raw only in Fastify middleware doesn't seem to be consistent. Also, frameworks usually expose convenient APIs in their request wrapper objects, so to me it makes more sense to allow user to deal with the familiar API (which is framework's request wrapper).


if (echoHeader) {
reply.header(headerName, requestId)
Expand Down Expand Up @@ -129,15 +129,15 @@ fastifyPlugin[Symbol.for('fastify.display-name')] = pluginName
const koaMiddleware = ({
useHeader = false,
headerName = 'X-Request-Id',
requestIdFactory = uuidv1,
requestIdFactory = null,
echoHeader = false
} = {}) => {
return (ctx, next) => {
let requestId
if (useHeader) {
requestId = ctx.request.headers[headerName.toLowerCase()]
}
requestId = requestId || requestIdFactory()
requestId = requestId || (requestIdFactory ? requestIdFactory(ctx.request) : uuidv1())

if (echoHeader) {
ctx.set(headerName, requestId)
Expand Down Expand Up @@ -166,15 +166,15 @@ const koaMiddleware = ({
const koaV1Middleware = ({
useHeader = false,
headerName = 'X-Request-Id',
requestIdFactory = uuidv1,
requestIdFactory = null,
echoHeader = false
} = {}) => {
return function * (next) {
let requestId
if (useHeader) {
requestId = this.request.headers[headerName.toLowerCase()]
}
requestId = requestId || requestIdFactory()
requestId = requestId || (requestIdFactory ? requestIdFactory(this.request) : uuidv1())

if (echoHeader) {
this.response.set(headerName, requestId)
Expand Down Expand Up @@ -202,7 +202,7 @@ const hapiPlugin = ({
const {
useHeader = false,
headerName = 'X-Request-Id',
requestIdFactory = uuidv1,
requestIdFactory = null,
echoHeader = false
} = options

Expand All @@ -211,7 +211,7 @@ const hapiPlugin = ({
if (useHeader) {
requestId = request.headers[headerName.toLowerCase()]
}
requestId = requestId || requestIdFactory()
requestId = requestId || (requestIdFactory ? requestIdFactory(request) : uuidv1())

als.enterWith(requestId)
wrapHttpEmitters(request.raw.req, request.raw.res)
Expand Down
22 changes: 22 additions & 0 deletions tests/express.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,28 @@ describe('cls-rtracer for Express', () => {
})
})

test('calls request id factory with req', () => {
const app = express()
const idFactory = req => req

app.use(rTracer.expressMiddleware({
requestIdFactory: idFactory
}))

app.get('/test', (req, res) => {
if (req === rTracer.id()) {
res.end()
} else {
res.status(500).end()
}
})

return request(app).get('/test')
.then(res => {
expect(res.statusCode).toBe(200)
})
})

test('ignores header by default', () => {
const app = express()
app.use(rTracer.expressMiddleware())
Expand Down
22 changes: 22 additions & 0 deletions tests/fastify.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,28 @@ describe('cls-rtracer for Fastify', () => {
})
})

test('calls request id factory with req', () => {
const app = Fastify()
const idFactory = req => req

app.register(rTracer.fastifyPlugin, {
requestIdFactory: idFactory
})

app.get('/test', async (request, reply) => {
if (request.raw === rTracer.id()) {
reply.send({})
} else {
reply.status(500)
}
})

return app.ready().then(() => request(app.server).get('/test'))
.then(res => {
expect(res.statusCode).toBe(200)
})
})

test('ignores header by default', () => {
const app = Fastify()
app.register(rTracer.fastifyPlugin)
Expand Down
24 changes: 24 additions & 0 deletions tests/fastifyv2.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,30 @@ for (const type of types) {
})
})

test('calls request id factory with req', () => {
const app = Fastify()
const idFactory = req => req

register(type, app, {
requestIdFactory: idFactory
})

app.get('/test', async (request, reply) => {
// console.log('request', request)
puzpuzpuz marked this conversation as resolved.
Show resolved Hide resolved
if (request.raw === rTracer.id()) {
reply.send({})
}
else {
puzpuzpuz marked this conversation as resolved.
Show resolved Hide resolved
throw new Error('Invalid')
}
})

return app.ready().then(() => request(app.server).get('/test'))
.then(res => {
expect(res.statusCode).toBe(200)
})
})

test('ignores header by default', () => {
const app = Fastify()
register(type, app)
Expand Down
24 changes: 24 additions & 0 deletions tests/hapi.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,30 @@ describe('cls-rtracer for Hapi', () => {
expect(res.result.id).toEqual(idFactory())
})

test('calls request id factory with req', async () => {
const idFactory = req => req

server = await setupServer({
options: {
requestIdFactory: idFactory
},
handler: request => {
if (request === rTracer.id()) {
return 'OK'
} else {
throw new Error('Not OK')
}
}
})

const res = await server.inject({
method: 'get',
url: '/'
})

expect(res.statusCode).toBe(200)
})

test('ignores header by default', async () => {
const idInHead = 'id-from-header'
let id
Expand Down
23 changes: 23 additions & 0 deletions tests/koa.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,29 @@ describe('cls-rtracer for Koa', () => {
})
})

test('calls request id factory with req', () => {
const app = new Koa()
const idFactory = req => req

app.use(rTracer.koaMiddleware({
requestIdFactory: idFactory
}))

app.use((ctx) => {
if (ctx.request === rTracer.id()) {
ctx.body = 'OK'
}
else {
puzpuzpuz marked this conversation as resolved.
Show resolved Hide resolved
throw new Error('Not OK')
}
})

return request(app.callback()).get('/')
.then(res => {
expect(res.statusCode).toBe(200)
})
})

test('ignores header by default', () => {
const app = new Koa()
app.use(rTracer.koaMiddleware())
Expand Down
23 changes: 23 additions & 0 deletions tests/koav1.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,29 @@ describe('cls-rtracer for Koa v1', () => {
})
})

test('calls request id factory with req', () => {
const app = new Koa()
const idFactory = req => req
app.use(rTracer.koaV1Middleware({
requestIdFactory: idFactory
}))

app.use(function * () {
if (this.request === rTracer.id()) {
this.body = 'OK'
}
else {
puzpuzpuz marked this conversation as resolved.
Show resolved Hide resolved
throw new Error('Not OK')
}
})

return request(app.callback()).get('/')
.then(res => {
expect(res.statusCode).toBe(200)
expect(res.body.id).toEqual(idFactory())
puzpuzpuz marked this conversation as resolved.
Show resolved Hide resolved
})
})

test('ignores header by default', () => {
const app = new Koa()
app.use(rTracer.koaV1Middleware())
Expand Down