-
Notifications
You must be signed in to change notification settings - Fork 117
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
The reqId type is overly-broad #268
Comments
Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests. |
The default numbering could also use a change. Otherwise use https://www.npmjs.com/package/hyperid or apply the same concept with a custom function to have zero additional dependencies. const {randomBytes} = require('crypto')
function createHyperId (bytes = 16) {
let count = 1
let id = randomBytes(bytes).toString('hex')
function hyperid () {
if (count < 1000000) return `${id}-${count++}`
return `${id = randomBytes(bytes).toString('hex')}-${count = 1, count++}`
}
return hyperid
}
module.exports = createHyperId |
Here's a PR for the string based request id generation: #268 |
The patching of the http IncomingMessage to add the
id
properly is arguably bad, but that discussion aside,reqId
is too broad. The inclusion ofnumber
andobject
seems odd, considering that this value should be serialisable as a string in a header.The text was updated successfully, but these errors were encountered: