feat(mojaloop/#3885): extend fxquote functionality to persistant mode #348
Conversation
src/model/fxQuotes.js
Outdated
| try { | ||
| // calculate a SHA-256 of the request | ||
| const hash = calculateRequestHash(fxQuoteResponse) | ||
| this.log.debug(`Calculated sha256 hash of quote response with id ${conversionRequestId} as: ${hash}`) |
There was a problem hiding this comment.
why not just:
this.log.debug('Calculated sha256 hash of quote response:', { conversionRequestId, hash })
There was a problem hiding this comment.
done. here and throughout.
src/model/fxQuotes.js
Outdated
| } | ||
| } catch (err) { | ||
| // internal-error | ||
| this.log.error(`Error in checkDuplicateFxQuoteResponse: ${getStackOrInspect(err)}`) |
There was a problem hiding this comment.
this.log.error('Error in checkDuplicateFxQuoteResponse: ', err)
This way we'll log much more useful information about an error (e.g.: message, stack, code, cause):
https://github.com/mojaloop/central-services-logger/blob/master/src/contextLogger.js#L131
There was a problem hiding this comment.
done. here and throughout
src/model/fxQuotes.js
Outdated
| this.log.debug(`Calculated sha256 hash of quote response with id ${conversionRequestId} as: ${hash}`) | ||
|
|
||
| const dupchk = await this.db.getFxQuoteResponseDuplicateCheck(conversionRequestId) | ||
| this.log.debug(`DB query for quote response duplicate check with id ${conversionRequestId} returned: ${util.inspect(dupchk)}`) |
There was a problem hiding this comment.
this.log.debug('DB query for fxQuote response duplicate check returned: ', { conversionRequestId, dupchk })
Or we we don't want to repeat conversionRequestId in all logs across the the method, we can create a child logger an the beginning, and use it through the rest of the method code:
async checkDuplicateFxQuoteResponse (conversionRequestId, fxQuoteResponse) {
const log = this.log.child({ conversionRequestId })
try {
....
log.debug('DB query for fxQuote response duplicate check returned: ', { dupchk })
// conversionRequestId will be added to logs output
| @@ -67,6 +67,14 @@ | |||
| } | |||
| } | |||
| }, | |||
| "PROXY_CACHE": { | |||
| "enabled": false, | |||
| "type": "redis", | |||
There was a problem hiding this comment.
shouldn't we use redis-cluster as it's default proxyCache type?
There was a problem hiding this comment.
let me do that in a follow up PR. I didn't want to touch the int harness.
src/data/database.js
Outdated
| this.writeLog(`inserted new fxQuote in db: ${util.inspect(newFxQuote)}`) | ||
| return newFxQuote | ||
| } catch (err) { | ||
| this.writeLog(`Error in createFxQuote: ${getStackOrInspect(err)}`) |
There was a problem hiding this comment.
I'd suggest stopping using this.writeLog to log error, coz it uses debug severity, so we won't have such logs in production
There was a problem hiding this comment.
yep. I overlooked this when merging in your changes.
src/model/fxQuotes.js
Outdated
| this.log.debug(`DB query for quote duplicate check with id ${fxQuoteRequest.conversionRequestId} returned: ${util.inspect(dupchk)}`) | ||
|
|
||
| if (!dupchk) { | ||
| // no existing record for this quoteId found |
There was a problem hiding this comment.
Personally, I prefer add log instead of such comment:
if (!dupchk) {
this.log.info('no existing record for this quoteId found')
return ...
so we'll have additional info in logs during debugging
| const envConfig = new Config() | ||
| if (!envConfig.simpleRoutingMode) { | ||
| // check if this is a resend or an erroneous duplicate | ||
| const dupe = await this.checkDuplicateFxQuoteRequest(fxQuoteRequest) |
There was a problem hiding this comment.
just wondering about naming: what is dupe?
There was a problem hiding this comment.
shorthand for an object that details whether a request id is a duplicate request and/or if a hash of the request has been seen before.
src/model/fxQuotes.js
Outdated
| // any-error | ||
| // as we are on our own in this context, dont just rethrow the error, instead... | ||
| // get the model to handle it | ||
| this.log.error(`Error forwarding quote request: ${getStackOrInspect(err)}. Attempting to send error callback to ${fspiopSource}`) |
There was a problem hiding this comment.
we can log just this part here:
this.log.error('Error forwarding fxQuote request: ', err)
coz this.handleException() already logs the next info inside itself:
async handleException (...) {
this.log.info('Attempting to send error callback to fspiopSource:', { conversionRequestId, fspiopSource })
...
|
kleyow
requested review from
mdebarros,
elnyry-sam-k,
oderayi,
vijayg10 and
bushjames
as code owners
No description provided.