Skip to content

Commit 169f4d4

Browse files
naorpelednaorpeled
committed
.
1 parent 786e9ee commit 169f4d4

File tree

5 files changed

+109
-67
lines changed

5 files changed

+109
-67
lines changed

‎__tests__/errorHandling.unit.js‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,52 @@ describe('Error Handling Tests:', function () {
373373
const error = new errors.ResponseError('test message');
374374
expect(error.code).toBe(500);
375375
});
376+
377+
it('ApiError with string message', async function () {
378+
let _event = Object.assign({}, event, { path: '/testError' });
379+
let result = await new Promise(r => api5.run(_event, {}, (e, res) => { r(res) }));
380+
expect(result).toEqual({
381+
multiValueHeaders: { 'content-type': ['application/json'] },
382+
statusCode: 500,
383+
body: '{"error":"This is a test error message"}',
384+
isBase64Encoded: false
385+
});
386+
});
387+
388+
it('ApiError with code and message', async function () {
389+
let _event = Object.assign({}, event, { path: '/testError' });
390+
let result = await new Promise(r => api4.run(_event, {}, (e, res) => { r(res) }));
391+
expect(result).toEqual({
392+
multiValueHeaders: {},
393+
statusCode: 500,
394+
body: 'this is an error: false',
395+
isBase64Encoded: false
396+
});
397+
});
398+
399+
it('ApiError with message and detail', async function () {
400+
let _event = Object.assign({}, event, { path: '/testErrorDetail' });
401+
let result = await new Promise(r => api5.run(_event, {}, (e, res) => { r(res) }));
402+
expect(result).toEqual({
403+
multiValueHeaders: { 'content-type': ['application/json'] },
404+
statusCode: 500,
405+
body: '{"error":"This is a test error message"}',
406+
isBase64Encoded: false
407+
});
408+
});
409+
410+
it('ApiError properties', function () {
411+
const error = new errors.ApiError('test message', 403, { foo: 'bar' });
412+
expect(error.name).toBe('ApiError');
413+
expect(error.message).toBe('test message');
414+
expect(error.code).toBe(403);
415+
expect(error.detail).toEqual({ foo: 'bar' });
416+
});
417+
418+
it('ApiError default code', function () {
419+
const error = new errors.ApiError('test message');
420+
expect(error.code).toBe(500);
421+
});
376422
})
377423

378424
describe('Logging', function () {

‎index.d.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,8 +366,13 @@ export declare class ConfigurationError extends Error {
366366
}
367367

368368
export declare class ResponseError extends Error {
369+
constructor(message: string, code: number);
370+
}
371+
372+
export declare class ApiError extends Error {
369373
constructor(message: string, code?: number, detail?: any);
370-
code: number;
374+
name: 'ApiError';
375+
code?: number;
371376
detail?: any;
372377
}
373378

‎index.js‎

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ const RESPONSE = require('./lib/response');
1010
const UTILS = require('./lib/utils');
1111
const LOGGER = require('./lib/logger');
1212
const S3 = () => require('./lib/s3-service');
13-
const { ResponseError, ConfigurationError } = require('./lib/errors');
13+
const { ResponseError, ConfigurationError, ApiError } = require('./lib/errors');
1414
const prettyPrint = require('./lib/prettyPrint');
1515

1616
class API {
@@ -42,8 +42,8 @@ class API {
4242
: {};
4343
this._compression =
4444
props &&
45-
(typeof props.compression === 'boolean' ||
46-
Array.isArray(props.compression))
45+
(typeof props.compression === 'boolean' ||
46+
Array.isArray(props.compression))
4747
? props.compression
4848
: false;
4949

@@ -84,7 +84,7 @@ class API {
8484
this._app = {};
8585

8686
// Executed after the callback
87-
this._finally = () => {};
87+
this._finally = () => { };
8888

8989
// Global error status (used for response parsing errors)
9090
this._errorStatus = 500;
@@ -213,8 +213,8 @@ class API {
213213
stack: _stack['m'][method]
214214
? _stack['m'][method].concat(stack)
215215
: _stack['*'][method]
216-
? _stack['*'][method].concat(stack)
217-
: stack,
216+
? _stack['*'][method].concat(stack)
217+
: stack,
218218
// inherited: _stack[method] ? _stack[method] : [],
219219
route: '/' + parsedPath.join('/'),
220220
path: '/' + this._prefix.concat(parsedPath).join('/'),
@@ -328,26 +328,21 @@ class API {
328328

329329
// Catch all async/sync errors
330330
async catchErrors(e, response, code, detail) {
331-
// Error messages should respect the app's base64 configuration
332331
response._isBase64 = this._isBase64;
333332

334-
// Strip the headers, keep whitelist
335333
const strippedHeaders = Object.entries(response._headers).reduce(
336334
(acc, [headerName, value]) => {
337335
if (!this._errorHeaderWhitelist.includes(headerName.toLowerCase())) {
338336
return acc;
339337
}
340-
341338
return Object.assign(acc, { [headerName]: value });
342339
},
343340
{}
344341
);
345342

346343
response._headers = Object.assign(strippedHeaders, this._headers);
347-
348344
let message;
349345

350-
// Set the status code
351346
response.status(code ? code : this._errorStatus);
352347

353348
let info = {
@@ -357,10 +352,9 @@ class API {
357352
stack: (this._logger.stack && e.stack) || undefined,
358353
};
359354

360-
const wasStringError =
361-
e instanceof ResponseError && e.originalMessage !== undefined;
355+
const isApiError = e instanceof ApiError;
362356

363-
if (e instanceof Error && !wasStringError) {
357+
if (e instanceof Error && !isApiError) {
364358
message = e.message;
365359
if (this._logger.errorLogging) {
366360
this.log.fatal(message, info);
@@ -372,28 +366,20 @@ class API {
372366
}
373367
}
374368

375-
// If first time through, process error middleware
376369
if (response._state === 'processing') {
377-
// Flag error state (this will avoid infinite error loops)
378370
response._state = 'error';
379-
380-
// Execute error middleware
381371
for (const err of this._errors) {
382372
if (response._state === 'done') break;
383-
// Promisify error middleware
384-
// TODO: using async within a promise is an antipattern, therefore we need to refactor this asap
385-
// eslint-disable-next-line no-async-promise-executor
386373
await new Promise(async (r) => {
387374
let rtn = await err(e, response._request, response, () => {
388375
r();
389376
});
390377
if (rtn) response.send(rtn);
391378
r();
392379
});
393-
} // end for
380+
}
394381
}
395382

396-
// Throw standard error unless callback has already been executed
397383
if (response._state !== 'done') response.json({ error: message });
398384
} // end catch
399385

@@ -453,8 +439,8 @@ class API {
453439
typeof args[0] === 'string'
454440
? Array.of(args.shift())
455441
: Array.isArray(args[0])
456-
? args.shift()
457-
: ['/*'];
442+
? args.shift()
443+
: ['/*'];
458444

459445
// Init middleware stack
460446
let middleware = [];

‎lib/errors.js‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,21 @@ class ConfigurationError extends Error {
3434
}
3535

3636
class ResponseError extends Error {
37-
constructor(message, code, detail) {
37+
constructor(message, code) {
3838
super(message);
3939
this.name = 'ResponseError';
40+
this.code = code;
41+
}
42+
}
43+
44+
class ApiError extends Error {
45+
constructor(message, code, detail) {
46+
super(message);
47+
this.name = 'ApiError';
4048
this.code = typeof code === 'number' ? code : 500;
4149
if (detail !== undefined) {
4250
this.detail = detail;
4351
}
44-
// Track if this error was created from a string message
45-
this.originalMessage = typeof message === 'string' ? message : undefined;
4652
}
4753
}
4854

@@ -60,5 +66,6 @@ module.exports = {
6066
MethodError,
6167
ConfigurationError,
6268
ResponseError,
63-
FileError,
69+
ApiError,
70+
FileError
6471
};

‎lib/response.js‎

Lines changed: 35 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ const UTILS = require('./utils.js');
1010
const fs = require('fs'); // Require Node.js file system
1111
const path = require('path'); // Require Node.js path
1212
const compression = require('./compression'); // Require compression lib
13-
const { ResponseError, FileError } = require('./errors'); // Require custom errors
13+
const { ResponseError, FileError, ApiError } = require('./errors'); // Require custom errors
1414

1515
// Lazy load AWS S3 service
1616
const S3 = () => require('./s3-service');
@@ -85,15 +85,15 @@ class RESPONSE {
8585
return asArr
8686
? this._headers
8787
: Object.keys(this._headers).reduce(
88-
(headers, key) =>
89-
Object.assign(headers, { [key]: this._headers[key].toString() }),
90-
{}
91-
); // return all headers
88+
(headers, key) =>
89+
Object.assign(headers, { [key]: this._headers[key].toString() }),
90+
{}
91+
); // return all headers
9292
return asArr
9393
? this._headers[key.toLowerCase()]
9494
: this._headers[key.toLowerCase()]
95-
? this._headers[key.toLowerCase()].toString()
96-
: undefined;
95+
? this._headers[key.toLowerCase()].toString()
96+
: undefined;
9797
}
9898

9999
// Issue #130
@@ -131,9 +131,9 @@ class RESPONSE {
131131

132132
this.header('Content-Type', 'application/json').send(
133133
(cb ? cb.replace(' ', '_') : 'callback') +
134-
'(' +
135-
this._serializer(body) +
136-
')'
134+
'(' +
135+
this._serializer(body) +
136+
')'
137137
);
138138
}
139139

@@ -193,8 +193,8 @@ class RESPONSE {
193193
typeof expires === 'function'
194194
? expires
195195
: typeof callback === 'function'
196-
? callback
197-
: (e) => {
196+
? callback
197+
: (e) => {
198198
if (e) this.error(e);
199199
};
200200

@@ -236,10 +236,10 @@ class RESPONSE {
236236
cookieString +=
237237
opts.maxAge && !isNaN(opts.maxAge)
238238
? '; MaxAge=' +
239-
((opts.maxAge / 1000) | 0) +
240-
(!opts.expires
241-
? '; Expires=' + new Date(Date.now() + opts.maxAge).toUTCString()
242-
: '')
239+
((opts.maxAge / 1000) | 0) +
240+
(!opts.expires
241+
? '; Expires=' + new Date(Date.now() + opts.maxAge).toUTCString()
242+
: '')
243243
: '';
244244

245245
// path (String): Path for the cookie
@@ -248,14 +248,14 @@ class RESPONSE {
248248
// secure (Boolean): Marks the cookie to be used with HTTPS only
249249
cookieString += opts.secure && opts.secure === true ? '; Secure' : '';
250250

251-
// sameSite (Boolean or String) Value of the “SameSite” Set-Cookie attribute
251+
// sameSite (Boolean or String) Value of the "SameSite" Set-Cookie attribute
252252
// see https://tools.ietf.org/html/draft-ietf-httpbis-cookie-same-site-00#section-4.1.1.
253253
cookieString +=
254254
opts.sameSite !== undefined
255255
? '; SameSite=' +
256-
(opts.sameSite === true
257-
? 'Strict'
258-
: opts.sameSite === false
256+
(opts.sameSite === true
257+
? 'Strict'
258+
: opts.sameSite === false
259259
? 'Lax'
260260
: opts.sameSite)
261261
: '';
@@ -323,7 +323,7 @@ class RESPONSE {
323323
let buffer, modified;
324324

325325
let opts = typeof options === 'object' ? options : {};
326-
let fn = typeof callback === 'function' ? callback : () => {};
326+
let fn = typeof callback === 'function' ? callback : () => { };
327327

328328
// Add optional parameter support
329329
if (typeof options === 'function') {
@@ -440,16 +440,16 @@ class RESPONSE {
440440
opts.methods
441441
? opts.methods
442442
: acam
443-
? acam
444-
: 'GET, PUT, POST, DELETE, OPTIONS'
443+
? acam
444+
: 'GET, PUT, POST, DELETE, OPTIONS'
445445
);
446446
this.header(
447447
'Access-Control-Allow-Headers',
448448
opts.headers
449449
? opts.headers
450450
: acah
451-
? acah
452-
: 'Content-Type, Authorization, Content-Length, X-Requested-With'
451+
? acah
452+
: 'Content-Type, Authorization, Content-Length, X-Requested-With'
453453
);
454454

455455
// Optional CORS headers
@@ -500,8 +500,8 @@ class RESPONSE {
500500
date && typeof date.toUTCString === 'function'
501501
? date
502502
: date && Date.parse(date)
503-
? new Date(date)
504-
: new Date();
503+
? new Date(date)
504+
: new Date();
505505
this.header('Last-Modified', lastModified.toUTCString());
506506
}
507507
return this;
@@ -559,10 +559,10 @@ class RESPONSE {
559559
},
560560
this._request.interface === 'alb'
561561
? {
562-
statusDescription: `${this._statusCode} ${UTILS.statusLookup(
563-
this._statusCode
564-
)}`,
565-
}
562+
statusDescription: `${this._statusCode} ${UTILS.statusLookup(
563+
this._statusCode
564+
)}`,
565+
}
566566
: {}
567567
);
568568

@@ -596,13 +596,11 @@ class RESPONSE {
596596
error(code, e, detail) {
597597
const message = typeof code !== 'number' ? code : e;
598598
const statusCode = typeof code === 'number' ? code : undefined;
599-
const errorDetail =
600-
typeof code !== 'number' && e !== undefined ? e : detail;
599+
const errorDetail = typeof code !== 'number' && e !== undefined ? e : detail;
601600

602-
const errorToSend =
603-
typeof message === 'string'
604-
? new ResponseError(message, statusCode, errorDetail)
605-
: message;
601+
const errorToSend = typeof message === 'string'
602+
? new ApiError(message, statusCode, errorDetail)
603+
: message;
606604

607605
this.app.catchErrors(errorToSend, this, statusCode, errorDetail);
608606
} // end error

0 commit comments

Comments
 (0)