Skip to content

Commit

Permalink
Using CourtbotError.wrap() to Streamline Code
Browse files Browse the repository at this point in the history 
* Using CourtbotError.wrap() to match simpler pattern for
getCaseParties()
* Capitalizing CourtbotError for style conventions
* Removing unnecessary imports
* Removing unnecessary unit tests
  • Loading branch information
@chriswsh
chriswsh committed May 5, 2017
1 parent ef44a7e commit 15b4a18
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 156 deletions.
83 changes: 31 additions & 52 deletions src/events.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const EventEmitter = require(`events`);
import courtbotError from './courtbotError';
import CourtbotError from './courtbotError';
import log4js from 'log4js';
import {COURTBOT_ERROR_NAME, COURTBOT_ERROR_TYPES} from './courtbotError';
import {COURTBOT_ERROR_TYPES} from './courtbotError';
import _ from "lodash";

class CourtbotEmitter extends EventEmitter {}
Expand Down Expand Up @@ -71,11 +71,11 @@ export function getCaseParties(casenumber, options) {
}
});

const wrappedErrors = _.map(errors, err => new courtbotError({ type: COURTBOT_ERROR_TYPES.API.GET, case: casenumber, timestamp: Date(), initialError: typeof err === 'object' ? err : {data: err} }));
const wrappedErrors = _.map(errors, err => CourtbotError.wrap(err, { type: COURTBOT_ERROR_TYPES.API.GET, casenumber: casenumber }));

if(errors.length) {
logger.error(wrappedErrors);
if(emitErrorEvent) emitter.emit("retrieve-parties-error", wrappedErrors);
if(emitErrorEvent) emitter.emit("retrieve-parties-error", wrappedErrors);
}

if(returnErrorsWithData) {
Expand All @@ -90,65 +90,44 @@ export function getCaseParties(casenumber, options) {
}

/* casenumber: string holding the casenumber to look up
* party: string holding the party name to look up
* errorMode: 2-bit integer that determines how errors are handled:
* 1s bit - whether the retrieve-parties-error event is emitted with error information
* 2s bit - whether the error information is also returned along with the parties found { parties: [], errors: [] }
* ========================================
* results: an array of {name: `partyname`}
* errors: an array of courtbotErrors
* party: string hold the party to look up
* options:
* emitErrorEvent - whether the retrieve-parties-error event is emitted with error information
* returnErrorsWithData - whether the error information is also returned along with the parties found { parties: [], errors: [] }
*/
export function getCasePartyEvents(casenumber, party, errorMode = 1) {
export function getCasePartyEvents(casenumber, party, options) {
const result = {
promises: []
}

const {emitErrorEvent, returnErrorsWithData} =
Object.assign(
{emitErrorEvent: true, returnErrorsWithData: false},
options
);

// emit runs synchronously because I/O is not involved, so result will always be populated
// before further functions are called.
emitter.emit("retrieve-party-events", casenumber, party, result);

let results = [];
let errors = [];

// instead of Promise.all(), use reduce() to catch all errors, but still return valid values
return result.promises.reduce((chain, dataPromise) => {
// First with glue all the promises together with error handling...
return chain.then(() => {
return dataPromise;
})
.then((foundParties) => {
results = results.concat(foundParties);
})
.catch((err) => {
logger.warn(`events.js getCasePartyEvents() data retrieval raw error on ` + Date() + `:`);
logger.warn(err);

// Wrap the errors if necessary
if (err.name !== COURTBOT_ERROR_NAME) {
let wrappedError = new courtbotError({ type: COURTBOT_ERROR_TYPES.API.GENERAL, case: casenumber, timestamp: Date(), initialError: typeof err === 'object' ? err : {data: err} });

errors = errors.concat(wrappedError);
}
else {
errors = errors.concat(err);
return wrapPromises(result.promises).then(results => {
const errors = results.errors;
const flattenedResults = _.flattenDeep(results.results);
const wrappedErrors = _.map(errors, err => CourtbotError.wrap(err, { type: COURTBOT_ERROR_TYPES.API.GET, casenumber: casenumber }));

if(errors.length) {
logger.error(wrappedErrors);
if (emitErrorEvent) emitter.emit(`retrieve-party-events-error`, wrappedErrors);
}

return true;
});
}, Promise.resolve())
// ... then we return the results
.then(() => {
// Emit the retrieve-parties-error first. Like retrieve-parties, it runs synchronously because there's no I/O
if (errorMode % 2) emitter.emit(`retrieve-party-events-error`, errors);

// Add the errors to the return value if dictated by errorMode
if ((errorMode >> 1) % 2) {
results = {
events: results,
errors: errors

if(returnErrorsWithData) {
return {
errors: wrappedErrors,
events: _.map([...flattenedResults], r => r)
}
} else {
return _.map([...flattenedResults], r => r);
}
}
return results;
});
}

Expand Down
135 changes: 31 additions & 104 deletions test/events.test.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import setup from './setup';
import courtbotError from '../src/courtbotError';
import { COURTBOT_ERROR_TYPES } from '../src/courtbotError';
import proxyquire from 'proxyquire';

Expand Down Expand Up @@ -179,43 +178,9 @@ describe(`events`, () => {
return testee.getCaseParties(dummyCase);
});

it(`should place the data from non-courtbotError non-objects into courtbotError.initialError.data`, () => {
let testData = `testing`;
it('should not emit the retrieve-parties-error event if the options.emitErrorEvent is false', () => {
emitter.on(`retrieve-parties-error`, retrieveErrorStub);

emitter.on(`retrieve-parties-error`, (errors) => {
expect(errors[0].initialError.data).to.equal(testData);
});

emitter.on(`retrieve-parties`, (casenumber, result) => {
dummyPartyList.forEach((elem) => {
result.promises.push(Promise.resolve(elem));
});

result.promises.push(Promise.reject(testData));
});

return testee.getCaseParties(dummyCase);
});

it(`should place the data from non-courtbotError objects into courtbotError.initialError`, () => {
let testData = new Error(`testing`);

emitter.on(`retrieve-parties-error`, (errors) => {
expect(errors[0].initialError).to.deep.equal(testData);
});

emitter.on(`retrieve-parties`, (casenumber, result) => {
dummyPartyList.forEach((elem) => {
result.promises.push(Promise.resolve(elem));
});

result.promises.push(Promise.reject(testData));
});

return testee.getCaseParties(dummyCase);
});

it('should not emit the retrieve-parties-error event if the 1s bit of errorMode is off', () => {
emitter.on(`retrieve-parties`, (casenumber, result) => {
dummyPartyList.forEach((elem) => {
result.promises.push(Promise.resolve(elem));
Expand All @@ -224,10 +189,13 @@ describe(`events`, () => {
result.promises.push(Promise.reject(`1`));
});

return expect(() => {testee.getCaseParties(dummyCase, 2)}).to.not.emitFrom(emitter, `retrieve-parties-error`);
return testee.getCaseParties(dummyCase, { emitErrorEvent: false })
.then(() => {
expect(retrieveErrorStub).to.not.have.been.called();
});
});

it('should return a { parties: [], errors: [] } object if the returnErrorsWithData property is true', () => {
it('should return a { parties: [], errors: [] } object if the options.returnErrorsWithData property is true', () => {
emitter.on(`retrieve-parties`, (casenumber, result) => {
dummyPartyList.forEach((elem) => {
result.promises.push(Promise.resolve(elem));
Expand All @@ -253,7 +221,7 @@ describe(`events`, () => {

emitter.on(`retrieve-parties-error`, (errors) => {
expect(errors[0].type).to.equal(COURTBOT_ERROR_TYPES.API.GET);
expect(errors[0].case).to.equal(dummyCase);
expect(errors[0].casenumber).to.equal(dummyCase);
});

return testee.getCaseParties(dummyCase);
Expand All @@ -266,20 +234,20 @@ describe(`events`, () => {

emitter.on(`retrieve-parties-error`, (errors) => {
expect(errors[0].type).to.equal(COURTBOT_ERROR_TYPES.API.GET);
expect(errors[0].case).to.equal(dummyCase);
expect(errors[0].casenumber).to.equal(dummyCase);
});

return testee.getCaseParties(dummyCase);
});

it('should emit a courtbotError of type COURTBOT_ERROR_TYPES.API.GET and the correct case number if a promise resolves with an array of objects, some of which do not have a "name" property. It should also count the number of malformed items.', () => {
it('should emit a courtbotError of type COURTBOT_ERROR_TYPES.API.GET and the correct case number if a promise resolves with an array of objects', () => {
emitter.on(`retrieve-parties`, (casenumber, result) => {
result.promises.push(Promise.resolve([{ notname: `a,b,c` }, dummyPartyList, { a: true }, { b: false }]));
});

emitter.on(`retrieve-parties-error`, (errors) => {
expect(errors[0].type).to.equal(COURTBOT_ERROR_TYPES.API.GET);
expect(errors[0].case).to.equal(dummyCase);
expect(errors[0].casenumber).to.equal(dummyCase);
});

return testee.getCaseParties(dummyCase);
Expand Down Expand Up @@ -318,7 +286,7 @@ describe(`events`, () => {
return expect(() => {testee.getCasePartyEvents(dummyCase, dummyParty)}).to.emitFrom(emitter, `retrieve-party-events`, dummyCase, dummyParty, emptyResult);
});

it(`if no errors in data retrieval, should return a concatenated array of all party names found`, () => {
it(`if no errors in data retrieval, should return an array of all events found`, () => {
emitter.on(`retrieve-party-events`, (casenumber, party, result) => {
dummyEventList.forEach((elem) => {
result.promises.push(Promise.resolve(elem));
Expand Down Expand Up @@ -353,7 +321,7 @@ describe(`events`, () => {
result.promises.push(Promise.reject(`3`));
});

// This test fails, but should be equivalent to the uncommented test. Will look into it later, but it works for now
// This test doesn't work as expected, but should be equivalent to the uncommented test. Will look into it later, but it works for now
// No sense wasting time
// return expect(() => { testee.getCasePartyEvents(dummyCase, dummyParty) }).to.emitFrom(emitter, `retrieve-party-events-error`);

Expand Down Expand Up @@ -383,61 +351,7 @@ describe(`events`, () => {
return testee.getCasePartyEvents(dummyCase);
});

it(`should place the data from non-courtbotError non-objects into courtbotError.initialError.data`, () => {
let testData = `testing`;

emitter.on(`retrieve-party-events-error`, (errors) => {
expect(errors[0].initialError.data).to.equal(testData);
});

emitter.on(`retrieve-party-events`, (casenumber, party, result) => {
dummyEventList.forEach((elem) => {
result.promises.push(Promise.resolve(elem));
});

result.promises.push(Promise.reject(testData));
});

return testee.getCasePartyEvents(dummyCase, dummyParty);
});

it(`should place the data from non-courtbotError objects into courtbotError.initialError`, () => {
let testData = new Error(`testing`);

emitter.on(`retrieve-party-events-error`, (errors) => {
expect(errors[0].initialError).to.deep.equal(testData);
});

emitter.on(`retrieve-party-events`, (casenumber, party, result) => {
dummyEventList.forEach((elem) => {
result.promises.push(Promise.resolve(elem));
});

result.promises.push(Promise.reject(testData));
});

return testee.getCaseParties(dummyCase, dummyParty);
});

it('should not place anything in intial error if the data error is a courtbotError', () => {
let testData = new courtbotError(``);

emitter.on(`retrieve-party-events-error`, (errors) => {
expect(errors[0].initialError).to.deep.equal(null);
});

emitter.on(`retrieve-party-events`, (casenumber, party, result) => {
dummyEventList.forEach((elem) => {
result.promises.push(Promise.resolve(elem));
});

result.promises.push(Promise.reject(testData));
});

return testee.getCasePartyEvents(dummyCase, dummyParty);
});

it('should not emit the retrieve-parties-error event if the 1s bit of errorMode is off', () => {
it('should not emit the retrieve-parties-error event if options.emitErrorEvent is false', () => {
emitter.on(`retrieve-party-events-error`, retrieveErrorStub);

emitter.on(`retrieve-party-events`, (casenumber, party, result) => {
Expand All @@ -448,13 +362,13 @@ describe(`events`, () => {
result.promises.push(Promise.reject(`1`));
});

return testee.getCasePartyEvents(dummyCase, dummyParty, 2)
return testee.getCasePartyEvents(dummyCase, dummyParty, { emitErrorEvent: false })
.then(() => {
expect(retrieveErrorStub).to.not.have.been.called();
});
});

it('should return a { events: [], errors: [] } object if the 2s bit of errorMode is on', () => {
it('should return a { events: [], errors: [] } object if options.returnErrorsWithData is true', () => {
emitter.on(`retrieve-party-events`, (casenumber, party, result) => {
dummyEventList.forEach((elem) => {
result.promises.push(Promise.resolve(elem));
Expand All @@ -465,13 +379,26 @@ describe(`events`, () => {
result.promises.push(Promise.reject(`3`));
});

return testee.getCasePartyEvents(dummyCase, dummyParty, 2)
return testee.getCasePartyEvents(dummyCase, dummyParty, { returnErrorsWithData: true })
.then((result) => {
expect(result.errors.every((e) => { return e.isCourtbotError === true })).to.equal(true);
expect(result.events).to.deep.equal(reducedDummyEventList);
});
});

it(`should emit a courtbotError of type COURBOT_ERROR_TYPES.API.GET and the correct case number if a promise rejects`, () => {
emitter.on(`retrieve-party-events`, (casenumber, party, result) => {
result.promises.push(Promise.reject(`fail`));
});

emitter.on(`retrieve-party-events-error`, (errors) => {
expect(errors[0].type).to.equal(COURTBOT_ERROR_TYPES.API.GET);
expect(errors[0].casenumber).to.equal(dummyCase);
});

return testee.getCasePartyEvents(dummyCase, dummyParty);
});

describe(`log4js`, () => {
it(`should send something to log4js when handling a rejected promise`, () => {
emitter.on(`retrieve-party-events`, (casenumber, party, result) => {
Expand All @@ -485,7 +412,7 @@ describe(`events`, () => {
});
});

describe(`sendNonReplyMessage`, () => {
describe(`sendNonReplyMessage`, () => {
let dummyTo, dummyMsg, dummyCommunicationType;

beforeEach(() => {
Expand Down

0 comments on commit 15b4a18

Please sign in to comment.