From 65e7880c81c4d3c1ce79f654b540bd2285257dda Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Mon, 28 Oct 2019 10:56:13 -0400 Subject: [PATCH] RatingService: Improved error handling This does only a slightly better job than before. --- src/server/service/RatingService.ts | 177 +++++++++++++---------- test/server/service/RatingServiceTest.ts | 111 ++++++++++++-- 2 files changed, 201 insertions(+), 87 deletions(-) diff --git a/src/server/service/RatingService.ts b/src/server/service/RatingService.ts index 0bf4fc8..690dcb2 100644 --- a/src/server/service/RatingService.ts +++ b/src/server/service/RatingService.ts @@ -34,6 +34,13 @@ import { UserResponse } from "../request/UserResponse"; type RequestCallback = () => void; +/** Result of rating */ +export type RateRequestResult = { + data: RateResult, + initialRatedDate: UnixTimestamp, + lastRatedDate: UnixTimestamp, +}; + /** * Handle rating requests @@ -93,32 +100,38 @@ export class RatingService _response: UserResponse, quote: ServerSideQuote, cmd: string, - ): Promise + ): Promise { - // cmd represents a request for a single rater - if ( !cmd && this._isQuoteValid( quote ) ) + const program = quote.getProgram(); + + return new Promise( resolve => { - // send an empty reply (keeps what is currently in the bucket) - this._server.sendResponse( request, quote, { - data: {}, - }, [] ); + // cmd represents a request for a single rater + if ( !cmd && this._isQuoteValid( quote ) ) + { + // send an empty reply (keeps what is currently in the + // bucket) + this._server.sendResponse( request, quote, { + data: {}, + }, [] ); - return Promise.resolve(); - } + // XXX: When this class is no longer responsible for + // sending the response to the server, this below data needs + // to represent the _current_ values, since as it is written + // now, it'll overwrite what is currently in the bucket + return resolve( { + data: { _unavailable_all: '0' }, + initialRatedDate: 0, + lastRatedDate: 0, + } ); + } - var program = quote.getProgram(); - - return new Promise( ( resolve, reject ) => + resolve( this._performRating( request, program, quote, cmd ) ); + } ) + .catch( err => { - try - { - this._performRating( request, program, quote, cmd, resolve ); - } - catch ( err ) - { - this._sendRatingError( request, quote, program, err ); - reject( err ); - } + this._sendRatingError( request, quote, program, err ); + throw err; } ); } @@ -155,67 +168,81 @@ export class RatingService } + /** + * Perform rating and process result + * + * @param request - user request to satisfy + * @param program - quote program + * @param quote - quote to process + * @param indv - individual supplier to rate (or empty) + * + * @return promise for results of rating + */ private _performRating( - request: UserRequest, - program: Program, - quote: ServerSideQuote, - indv: string, - c: RequestCallback, - ) + request: UserRequest, + program: Program, + quote: ServerSideQuote, + indv: string, + ): Promise { - var rater = this._rater_manager.byId( program.getId() ); + return new Promise( ( resolve, reject ) => + { + var rater = this._rater_manager.byId( program.getId() ); - this._logger.log( this._logger.PRIORITY_INFO, - "Performing '%s' rating for quote #%s", - quote.getProgramId(), - quote.getId() - ); + this._logger.log( this._logger.PRIORITY_INFO, + "Performing '%s' rating for quote #%s", + quote.getProgramId(), + quote.getId() + ); - rater.rate( quote, request.getSession(), indv, - ( rate_data: RateResult, actions: ClientActions ) => - { - actions = actions || []; - - this.postProcessRaterData( - request, rate_data, actions, program, quote - ); - - const class_dest = {}; - - const cleaned = this._cleanRateData( - rate_data, - class_dest - ); - - // TODO: move me during refactoring - this._dao.saveQuoteClasses( - quote, class_dest, () => {}, () => {} - ); - - // save all data server-side (important: do after - // post-processing); async - this._saveRatingData( quote, rate_data, indv, function() + rater.rate( quote, request.getSession(), indv, + ( rate_data: RateResult, actions: ClientActions ) => { - // we're done - c(); - } ); + actions = actions || []; - // no need to wait for the save; send the response - this._server.sendResponse( request, quote, { - data: cleaned, - initialRatedDate: quote.getRatedDate(), - lastRatedDate: quote.getLastPremiumDate() - }, actions ); - }, - ( message: string ) => - { - this._sendRatingError( request, quote, program, - Error( message ) - ); + this.postProcessRaterData( + request, rate_data, actions, program, quote + ); - c(); - } - ); + const class_dest = {}; + + const cleaned = this._cleanRateData( + rate_data, + class_dest + ); + + // TODO: move me during refactoring + this._dao.saveQuoteClasses( + quote, class_dest, () => {}, () => {} + ); + + const result = { + data: cleaned, + initialRatedDate: quote.getRatedDate(), + lastRatedDate: quote.getLastPremiumDate() + }; + + // save all data server-side (important: do after + // post-processing); async + this._saveRatingData( quote, rate_data, indv, function() + { + // we're done + resolve( result ); + } ); + + // no need to wait for the save; send the response + this._server.sendResponse( request, quote, result, actions ); + }, + ( message: string ) => + { + this._sendRatingError( request, quote, program, + Error( message ) + ); + + reject( Error( message ) ); + } + ); + } ); } diff --git a/test/server/service/RatingServiceTest.ts b/test/server/service/RatingServiceTest.ts index c9bb108..fd659d6 100644 --- a/test/server/service/RatingServiceTest.ts +++ b/test/server/service/RatingServiceTest.ts @@ -44,6 +44,31 @@ chai_use( require( 'chai-as-promised' ) ); describe( 'RatingService', () => { + it( "returns rating results", () => + { + const { + logger, + server, + raters, + dao, + request, + response, + quote, + stub_rate_data, + } = getStubs(); + + const sut = new Sut( logger, dao, server, raters ); + + const expected = { + data: stub_rate_data, + initialRatedDate: quote.getRatedDate(), + lastRatedDate: quote.getLastPremiumDate(), + }; + + return expect( sut.request( request, response, quote, "" ) ) + .to.eventually.deep.equal( expected ); + } ); + it( "saves rate data to own field", () => { const { @@ -86,17 +111,18 @@ describe( 'RatingService', () => } ); - it( "rejects with error", () => + it( "rejects and responds with error", () => { const { - logger, - server, - raters, dao, - request, - response, + logger, + program, quote, rater, + raters, + request, + response, + server, } = getStubs(); const expected_error = new Error( "expected error" ); @@ -105,8 +131,66 @@ describe( 'RatingService', () => const sut = new Sut( logger, dao, server, raters ); + let logged = false; + + logger.log = function( + priority: number, + _format: string, + qid: QuoteId, + program_id: string, + message: string, + ) + { + if ( typeof message === 'string' ) + { + expect( priority ).to.equal( logger.PRIORITY_ERROR ); + expect( qid ).to.equal( quote.getId() ); + expect( program_id ).to.equal( program.getId() ); + expect( message ).to.contain( expected_error.message ); + + logged = true; + } + + return logger; + }; + return expect( sut.request( request, response, quote, "" ) ) - .to.eventually.rejectedWith( expected_error ); + .to.eventually.rejectedWith( expected_error ) + .then( () => expect( logged ).to.be.true ); + } ); + + + it( "returns error message from rater", () => + { + const { + dao, + logger, + quote, + rater, + raters, + request, + response, + server, + } = getStubs(); + + const expected_message = 'expected foo'; + + const sut = new Sut( logger, dao, server, raters ); + + rater.rate = ( + _quote: ServerSideQuote, + _session: UserSession, + _indv: string, + _success: ( data: RateResult, actions: ClientActions ) => void, + failure: ( message: string ) => void, + ) => + { + failure( expected_message ); + return rater; + }; + + return expect( sut.request( request, response, quote, "" ) ) + .to.eventually.rejectedWith( Error, expected_message ); } ); @@ -142,7 +226,7 @@ describe( 'RatingService', () => } }( logger, dao, server, raters ); - return sut.request( request, response, quote, 'something' ); + sut.request( request, response, quote, 'something' ); } ); it( "calls getLastPremiumDate during #_performRating", done => @@ -183,9 +267,8 @@ describe( 'RatingService', () => return server; }; - return sut.request( request, response, quote, "" ); + sut.request( request, response, quote, "" ); } ); - } ); } ); @@ -211,9 +294,13 @@ function getStubs() _session: UserSession, _indv: string, success: ( data: RateResult, actions: ClientActions ) => void, + _failure: ( message: string ) => void, ) { - success( stub_rate_data, [] ); + // force to be async so that the tests resemble how the code + // actually runs + process.nextTick( () => success( stub_rate_data, [] ) ); + return this; } }; @@ -230,7 +317,7 @@ function getStubs() readonly PRIORITY_INFO: number = 3; readonly PRIORITY_SOCKET: number = 4; - log(): this + log( _priority: number, ..._args: Array ): this { return this; }