-
Notifications
You must be signed in to change notification settings - Fork 10
Conversation
src/createError.js
Outdated
const _createError = require('http-errors'); | ||
const { errors } = require('@elastic/elasticsearch'); | ||
|
||
module.exports.createError = (...args) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
module.exports = (...args) => {
src/createError.js
Outdated
for (let i = 0; i < args.length; i++) { | ||
let arg = args[i]; | ||
let type = typeof arg; | ||
if (type === 'object' && arg instanceof errors.ResponseError) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add comments please to indicate what each path is doing in this if.
src/index.js
Outdated
@@ -4,7 +4,7 @@ const { createRouter } = require('./app'); | |||
module.exports.Satellite = require('./satellite'); | |||
module.exports.logger = require('./logger'); | |||
module.exports.hash = require('./hash'); | |||
module.exports.createError = require('http-errors'); | |||
module.exports.createError = require('./createError').createError; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
module.exports.createError = require('./create-error');
NOTE: always name files in lower-case, since not all filesystems are case sensitive.
src/createError.js
Outdated
@@ -0,0 +1,22 @@ | |||
const _createError = require('http-errors'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename this file to src/create-error.js
95fb1e2
to
3403e96
Compare
Changed the file name, added comments, and changed the test('errors created without a status code should return a status of 500', () => {
expect(createError('testing').status).toBe(500);
}); |
Follow up of #63
Gave createError its own module so we could add cases for ElasticSearch errors.
Updated the tests to reflect the changes as well.
To test:
pnpm test
For this change, the ES error will be created into an
http-error
indirectly first, before being sent intocreateError
again as a nested error with additional props.Doing so this way, we get to keep the ES error
statusCode
, it won't be overwritten.However, I have no way of changing the description name of the final error to whatever ES might have named the specific status code to. The names are based on the
status
code thatcreateError
receives.For example, a
404
status code will always be created as aNotFoundError
, as shown belowThe log would be something like
So, it might be up for discussion if we want to keep the ES error status codes, or if we have it so they can be overwritten.