Skip to content

Commit

Permalink
feat: Add support for promises in handlers (#30)
Browse files Browse the repository at this point in the history
  • Loading branch information
@onebytegone @jthomerson
onebytegone authored and jthomerson committed Mar 21, 2019
1 parent c739611 commit 49f500c
Show file tree
Hide file tree
Showing 5 changed files with 241 additions and 12 deletions.
6 changes: 6 additions & 0 deletions src/interfaces.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ export interface RequestProcessor {
* processing. Failure to either send a response or call `next` will result in a hung
* process.
*
* If this function returns a Promise object, a `catch` method will automatically be
* attached to the promise. If the promise is rejected, that `catch` method will call
* `next`, passing along the rejected value or an error if the value is empty. If the
* returned promise is resolved, `next` will *not* be called automatically. It is up to
* the handler to call `next` when appropriate.
*
* @param req The request to be handled
* @param resp The response that will be sent when the request-handling process is
* complete
Expand Down
6 changes: 6 additions & 0 deletions src/utils/common-types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,9 @@ export function isStringArrayOfStringsMap(o: any): o is StringArrayOfStringsMap
return memo && _.isString(k) && isArrayOfStrings(v);
}, true);
}

export function isPromise(o: any): o is Promise<unknown> {
return o
&& typeof o === 'object'
&& typeof o.then === 'function';
}
19 changes: 17 additions & 2 deletions src/utils/wrapRequestProcessor.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import _ from 'underscore';
import { AnyRequestProcessor, NextCallback, ErrorHandlingRequestProcessor } from '../interfaces';
import { Request, Response } from '..';
import { isPromise } from './common-types';

function isErrorHandler(rh: AnyRequestProcessor): rh is ErrorHandlingRequestProcessor {
return rh.length === 4;
Expand All @@ -10,7 +11,15 @@ export function wrapRequestProcessor(rp: AnyRequestProcessor): ErrorHandlingRequ
if (isErrorHandler(rp)) {
return (err: unknown, req: Request, resp: Response, next: NextCallback) => {
if (err) {
return rp(err, req, resp, next);
const returned: any = rp(err, req, resp, next);

if (isPromise(returned)) {
returned.then(null, (newErr: unknown) => {
next(newErr || new Error('Rejected promise'));
});
}

return;
}

// Error handlers should not get invoked if there is no error, so we simply
Expand All @@ -27,7 +36,13 @@ export function wrapRequestProcessor(rp: AnyRequestProcessor): ErrorHandlingRequ
return next(err);
}

rp(req, resp, next);
const returned: any = rp(req, resp, next);

if (isPromise(returned)) {
returned.then(null, (newErr: unknown) => {
next(newErr || new Error('Rejected promise'));
});
}
};
}

Expand Down
184 changes: 176 additions & 8 deletions tests/chains/ProcessorChain.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,17 @@ describe('ProcessorChain', () => {
return new ProcessorChain(wrapRequestProcessors(procs));
};

const makeDoneFn = (endTest: Mocha.Done, tests: (args: any[]) => void): (() => void) => {
return (...args) => {
try {
tests(args);
endTest();
} catch(err) {
endTest(err);
}
};
};

it('calls processors in correct order - no error handlers and no errors', () => {
const procs: SinonSpy[] = [
/* 0 */ makeRequestProcessor('mw1'),
Expand All @@ -54,20 +65,39 @@ describe('ProcessorChain', () => {
assert.calledWithExactly(done); // `done` called with no args
});

it('does not call error handlers when no errors', () => {
it('calls async processors returning promises in correct order - no error handlers and no errors', (endTest) => {
const procs: SinonSpy[] = [
/* 0 */ makeRequestProcessor('mw1', { returnsResolvedPromise: true }),
/* 1 */ makeRequestProcessor('mw2'),
/* 2 */ makeRequestProcessor('mw3', { returnsResolvedPromise: true }),
/* 3 */ makeRequestProcessor('rh1'),
/* 4 */ makeRequestProcessor('rh2'),
/* 5 */ makeRequestProcessor('rh3', { returnsResolvedPromise: true }),
];

makeChain(procs).run(undefined, req, resp, makeDoneFn(endTest, (args) => {
assertAllCalledOnceInOrder(...procs);
expect(args).to.eql([]); // callback was called with no args
}));
});

it('does not call error handlers when no errors', (endTest) => {
const procs: SinonSpy[] = [
/* 0 */ makeRequestProcessor('mw1'),
/* 1 */ makeRequestProcessor('rh1'),
/* 2 */ makeRequestProcessor('eh1', { handlesErrors: true }),
/* 3 */ makeRequestProcessor('mw2'),
/* 4 */ makeRequestProcessor('rh2'),
/* 5 */ makeRequestProcessor('eh2', { handlesErrors: true }),
/* 5 */ makeRequestProcessor('eh2', { handlesErrors: true, returnsResolvedPromise: true }),
/* 6 */ makeRequestProcessor('rh3'),
/* 7 */ makeRequestProcessor('eh3', { handlesErrors: true }),
];

makeChain(procs).run(undefined, req, resp, done);
assertAllCalledOnceInOrder(procs[0], procs[1], procs[3], procs[4], done);
assertNotCalled(procs[2], procs[5]);
assert.calledWithExactly(done); // `done` called with no args
makeChain(procs).run(undefined, req, resp, makeDoneFn(endTest, (args) => {
assertAllCalledOnceInOrder(procs[0], procs[1], procs[3], procs[4], procs[6]);
assertNotCalled(procs[2], procs[5], procs[7]);
expect(args).to.eql([]); // callback was called with no args
}));
});

it('skips to error handlers on first thrown error', () => {
Expand All @@ -88,7 +118,26 @@ describe('ProcessorChain', () => {
assert.calledWithExactly(done); // `done` called with no args
});

it('calls subsequent error handlers when thrown error passed on (thrown error)', () => {
it('skips to error handlers on first rejected promise', (endTest) => {
const procs: SinonSpy[] = [
/* 0 */ makeRequestProcessor('mw1'),
/* 1 */ makeRequestProcessor('mw2', { returnsRejectedPromise: true }),
/* 2 */ makeRequestProcessor('rh1'),
/* 3 */ makeRequestProcessor('rh2'),
/* 4 */ makeRequestProcessor('eh1', { handlesErrors: true }),
// since the first error handler does not pass the error on when it calls `next`,
// this second error handler will not be called
/* 5 */ makeRequestProcessor('eh2', { handlesErrors: true }),
];

makeChain(procs).run(undefined, req, resp, makeDoneFn(endTest, (args) => {
assertAllCalledOnceInOrder(procs[0], procs[1], procs[4]);
assertNotCalled(procs[2], procs[3], procs[5]);
expect(args).to.eql([]); // callback was called with no args
}));
});

it('calls subsequent error handlers when thrown error is passed on (thrown error)', () => {
const procs: SinonSpy[] = [
/* 0 */ makeRequestProcessor('mw1'),
/* 1 */ makeRequestProcessor('mw2', { throwsError: true }),
Expand All @@ -104,6 +153,103 @@ describe('ProcessorChain', () => {
assertCalledWith(done, 'Error from "mw2"', true); // `done` called with Error(string)
});

it('skips to error handlers on first rejected promise (empty rejection)', (endTest) => {
const procs: SinonSpy[] = [
/* 0 */ makeRequestProcessor('mw1'),
/* 1 */ makeRequestProcessor('mw2', { returnsEmptyRejectedPromise: true }),
/* 2 */ makeRequestProcessor('rh1'),
/* 3 */ makeRequestProcessor('rh2'),
/* 4 */ makeRequestProcessor('eh1', { handlesErrors: true }),
// since the first error handler does not pass the error on when it calls `next`,
// this second error handler will not be called
/* 5 */ makeRequestProcessor('eh2', { handlesErrors: true }),
];

makeChain(procs).run(undefined, req, resp, makeDoneFn(endTest, (args) => {
assertAllCalledOnceInOrder(procs[0], procs[1], procs[4]);
assertNotCalled(procs[2], procs[3], procs[5]);
expect(args).to.eql([]); // callback was called with no args
}));
});

it('calls subsequent error handlers when a rejected promise\'s error is passed on', (endTest) => {
const procs: SinonSpy[] = [
/* 0 */ makeRequestProcessor('mw1'),
/* 1 */ makeRequestProcessor('mw2', { returnsRejectedPromise: true }),
/* 2 */ makeRequestProcessor('rh1'),
/* 3 */ makeRequestProcessor('rh2'),
/* 4 */ makeRequestProcessor('eh1', { handlesErrors: true, passesErrorToNext: true }),
/* 5 */ makeRequestProcessor('eh2', { handlesErrors: true, passesErrorToNext: true }),
];

makeChain(procs).run(undefined, req, resp, makeDoneFn(endTest, (args) => {
assertAllCalledOnceInOrder(procs[0], procs[1], procs[4], procs[5]);
assertNotCalled(procs[2], procs[3]);
// callback called with error string (not Error instance)
expect(args).to.have.length(1);
expect(args[0]).to.eql('Rejection from "mw2"');
}));
});

it('calls subsequent error handlers with default error from empty rejected promise', (endTest) => {
const procs: SinonSpy[] = [
/* 0 */ makeRequestProcessor('mw1'),
/* 1 */ makeRequestProcessor('mw2', { returnsEmptyRejectedPromise: true }),
/* 2 */ makeRequestProcessor('rh1'),
/* 3 */ makeRequestProcessor('rh2'),
/* 4 */ makeRequestProcessor('eh1', { handlesErrors: true, passesErrorToNext: true }),
/* 5 */ makeRequestProcessor('eh2', { handlesErrors: true, passesErrorToNext: true }),
];

makeChain(procs).run(undefined, req, resp, makeDoneFn(endTest, (args) => {
assertAllCalledOnceInOrder(procs[0], procs[1], procs[4], procs[5]);
assertNotCalled(procs[2], procs[3]);
// callback called with Error(string)
expect(args).to.have.length(1);
expect(args[0]).to.be.an.instanceOf(Error);
expect(args[0].message).to.eql('Rejected promise');
}));
});

it('calls subsequent error handlers with error from rejected promise', (endTest) => {
const procs: SinonSpy[] = [
/* 0 */ makeRequestProcessor('mw1'),
/* 1 */ makeRequestProcessor('mw2', { throwsError: true }),
/* 2 */ makeRequestProcessor('rh1'),
/* 3 */ makeRequestProcessor('rh2'),
/* 4 */ makeRequestProcessor('eh1', { handlesErrors: true, returnsRejectedPromise: true }),
/* 5 */ makeRequestProcessor('eh2', { handlesErrors: true, passesErrorToNext: true }),
];

makeChain(procs).run(undefined, req, resp, makeDoneFn(endTest, (args) => {
assertAllCalledOnceInOrder(procs[0], procs[1], procs[4], procs[5]);
assertNotCalled(procs[2], procs[3]);
// callback called with error string (not Error instance)
expect(args).to.have.length(1);
expect(args[0]).to.eql('Rejection from "eh1"');
}));
});

it('calls subsequent error handlers with default error from empty rejected promise', (endTest) => {
const procs: SinonSpy[] = [
/* 0 */ makeRequestProcessor('mw1'),
/* 1 */ makeRequestProcessor('mw2', { throwsError: true }),
/* 2 */ makeRequestProcessor('rh1'),
/* 3 */ makeRequestProcessor('rh2'),
/* 4 */ makeRequestProcessor('eh1', { handlesErrors: true, returnsEmptyRejectedPromise: true }),
/* 5 */ makeRequestProcessor('eh2', { handlesErrors: true, passesErrorToNext: true }),
];

makeChain(procs).run(undefined, req, resp, makeDoneFn(endTest, (args) => {
assertAllCalledOnceInOrder(procs[0], procs[1], procs[4], procs[5]);
assertNotCalled(procs[2], procs[3]);
// callback called with Error(string)
expect(args).to.have.length(1);
expect(args[0]).to.be.an.instanceOf(Error);
expect(args[0].message).to.eql('Rejected promise');
}));
});

it('skips to error handlers on first non-thrown error', () => {
const procs: SinonSpy[] = [
/* 0 */ makeRequestProcessor('mw1'),
Expand All @@ -122,7 +268,7 @@ describe('ProcessorChain', () => {
assert.calledWithExactly(done); // `done` called with no args
});

it('calls subsequent error handlers when thrown error passed on (non-thrown error)', () => {
it('calls subsequent error handlers when thrown error is passed on (non-thrown error)', () => {
const procs: SinonSpy[] = [
/* 0 */ makeRequestProcessor('mw1'),
/* 1 */ makeRequestProcessor('mw2', { callsNextWithError: true }),
Expand Down Expand Up @@ -159,6 +305,28 @@ describe('ProcessorChain', () => {
assert.calledWithExactly(done); // `done` called with no args
});

it('resumes processors after error handler handles rejected promise', (endTest) => {
const procs: SinonSpy[] = [
/* 0 */ makeRequestProcessor('mw1'),
/* 1 */ makeRequestProcessor('mw2', { returnsRejectedPromise: true }),
/* 2 */ makeRequestProcessor('rh1'),
/* 3 */ makeRequestProcessor('rh2'),
/* 4 */ makeRequestProcessor('eh1', { handlesErrors: true }),
// since the previous error handler (eh1) did not pass the error on to the next,
// then the next error handler (eh2) will not get called
/* 5 */ makeRequestProcessor('eh2', { handlesErrors: true }),
// but these regular processor (route handler / middleware) will get called
/* 6 */ makeRequestProcessor('rh3'),
/* 7 */ makeRequestProcessor('rh4'),
];

makeChain(procs).run(undefined, req, resp, makeDoneFn(endTest, (args) => {
assertAllCalledOnceInOrder(procs[0], procs[1], procs[4], procs[6], procs[7]);
assertNotCalled(procs[2], procs[3], procs[5]);
expect(args).to.eql([]); // callback was called with no args
}));
});

it('short-circuits to done when next(\'route\') called', () => {
const procs: SinonSpy[] = [
/* 0 */ makeRequestProcessor('mw1'),
Expand Down
38 changes: 36 additions & 2 deletions tests/test-utils/makeRequestProcessor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ interface MakeFunctionOpts {
callsNextWithError?: boolean;
callsNextRoute?: boolean;
passesErrorToNext?: boolean;
returnsResolvedPromise?: boolean;
returnsRejectedPromise?: boolean;
returnsEmptyRejectedPromise?: boolean;
}

const DEFAULT_OPTS: MakeFunctionOpts = {
Expand All @@ -19,8 +22,19 @@ const DEFAULT_OPTS: MakeFunctionOpts = {
callsNextWithError: false,
callsNextRoute: false,
passesErrorToNext: false,
returnsResolvedPromise: false,
returnsRejectedPromise: false,
returnsEmptyRejectedPromise: false,
};

function delay(value: unknown, ms: number = 4): Promise<unknown> {
return new Promise((resolve) => {
setTimeout(() => {
resolve(value);
}, ms);
});
}

/**
* When using sinon assertions (especially for asserting that many functions were called,
* and that call order was correct), it's best to have named functions. So, this function
Expand All @@ -33,7 +47,7 @@ export default function makeRequestProcessor(name: string, userOpts?: MakeFuncti
rp: AnyRequestProcessor;

if (opts.handlesErrors) {
rp = (err: unknown, _req: Request, _resp: Response, next: NextCallback): void => {
rp = (err: unknown, _req: Request, _resp: Response, next: NextCallback): void | Promise<unknown> => {
if (opts.throwsError) {
throw new Error(`Error from "${name}"`);
} else if (opts.callsNextWithError) {
Expand All @@ -42,6 +56,16 @@ export default function makeRequestProcessor(name: string, userOpts?: MakeFuncti
return next('route');
} else if (opts.passesErrorToNext) {
return next(err);
} else if (opts.returnsRejectedPromise) {
return delay(Promise.reject(`Rejection from "${name}"`));
} else if (opts.returnsEmptyRejectedPromise) {
return delay(Promise.reject());
} else if (opts.returnsResolvedPromise && opts.callsNext) {
return delay(undefined).then(() => {
next(); // eslint-disable-line callback-return
});
} else if (opts.returnsResolvedPromise) {
return delay(Promise.resolve());
} else if (opts.callsNext) {
return next();
}
Expand All @@ -50,13 +74,23 @@ export default function makeRequestProcessor(name: string, userOpts?: MakeFuncti
if (opts.passesErrorToNext) {
throw new Error(`Invalid makeFunction options: ${JSON.stringify(opts)}`);
}
rp = (_req: Request, _resp: Response, next: NextCallback): void => {
rp = (_req: Request, _resp: Response, next: NextCallback): void | Promise<unknown> => {
if (opts.throwsError) {
throw new Error(`Error from "${name}"`);
} else if (opts.callsNextWithError) {
return next(`Error from "${name}"`);
} else if (opts.callsNextRoute) {
return next('route');
} else if (opts.returnsRejectedPromise) {
return delay(Promise.reject(`Rejection from "${name}"`));
} else if (opts.returnsEmptyRejectedPromise) {
return delay(Promise.reject());
} else if (opts.returnsResolvedPromise && opts.callsNext) {
return delay(undefined).then(() => {
next(); // eslint-disable-line callback-return
});
} else if (opts.returnsResolvedPromise) {
return delay(Promise.resolve());
} else if (opts.callsNext) {
return next();
}
Expand Down

0 comments on commit 49f500c

Please sign in to comment.