From 29f23dcf64d6b72a7d1d96c6baf8b70ff5f1c5bf Mon Sep 17 00:00:00 2001 From: Gadi Cohen Date: Sat, 6 Feb 2021 15:36:01 +0200 Subject: [PATCH] fix(modules): don't log errors when {validateResult:false} (#16) If { validateResult: false } we already don't throw from earlier fix. However, we still logged the errors, which doesn't really make sense. Although the user could turn off error logging in general for all requests, this fix will turn off error logging "automatically" just for the specific request where { validateResult: false }. --- src/lib/moduleExec.spec.ts | 48 ++++++++++++++++++++++++++++++++++++++ src/lib/moduleExec.ts | 14 +++++++++-- src/modules/search.spec.ts | 9 ------- 3 files changed, 60 insertions(+), 11 deletions(-) create mode 100644 src/lib/moduleExec.spec.ts diff --git a/src/lib/moduleExec.spec.ts b/src/lib/moduleExec.spec.ts new file mode 100644 index 00000000..8f2f6fd3 --- /dev/null +++ b/src/lib/moduleExec.spec.ts @@ -0,0 +1,48 @@ +import search from '../modules/search'; +const { InvalidOptionsError } = require('./errors'); +import moduleExec from './moduleExec'; + +import _env from '../env-node'; +import _fetch from './yahooFinanceFetch'; +import _moduleExec from './moduleExec'; + +const yf = { + _env, + _fetch, + _opts: { validation: { logErrors: true }}, + _moduleExec, + search +}; + +describe('moduleExec', () => { + + describe('result validation', () => { + + it('throws on unexpected input', async () => { + yf._opts.validation.logErrors = false; + await expect(yf.search('AAPL', {}, { devel: 'search-fakeBadResult.json' })) + .rejects.toThrow(/Failed Yahoo Schema/) + yf._opts.validation.logErrors = true; + }); + + it('dont throw or log on unexpected input with {validateResult: false}', async () => { + yf._opts.validation.logErrors = true; + const realConsole = console; + const fakeConsole = { error: jest.fn(), log: jest.fn(), dir: jest.fn() }; + + /* @ts-ignore */ + console = fakeConsole; + await expect(yf.search('AAPL', {}, { + devel: 'search-fakeBadResult.json', + validateResult: false + })).resolves.toBeDefined(); + console = realConsole; + + expect(fakeConsole.log).not.toHaveBeenCalled(); + expect(fakeConsole.error).not.toHaveBeenCalled(); + expect(fakeConsole.dir).not.toHaveBeenCalled(); + }); + + }); + +}); diff --git a/src/lib/moduleExec.ts b/src/lib/moduleExec.ts index 1ff47159..5e00d136 100644 --- a/src/lib/moduleExec.ts +++ b/src/lib/moduleExec.ts @@ -128,6 +128,16 @@ export default async function moduleExec(this: ThisWithFetch, opts: ModuleExecOp if (opts.result.transformWith) result = opts.result.transformWith(result); + const validateResult = !moduleOpts + || moduleOpts.validateResult === undefined + || moduleOpts.validateResult === true; + + const validationOpts = { + ...this._opts?.validation, + // Set logErrors=false if validateResult=false + logErrors: validateResult ? this._opts?.validation?.logErrors : false, + }; + /* * Validate the returned result (after transforming, above) and coerce types. * @@ -146,9 +156,9 @@ export default async function moduleExec(this: ThisWithFetch, opts: ModuleExecOp * database, etc. Otherwise you'll receive an error. */ try { - validateAndCoerceTypes(result, opts.result.schemaKey, undefined, this._opts?.validation); + validateAndCoerceTypes(result, opts.result.schemaKey, undefined, validationOpts); } catch (error) { - if (!moduleOpts || moduleOpts.validateResult === undefined || moduleOpts.validateResult === true) + if (validateResult) throw error; } diff --git a/src/modules/search.spec.ts b/src/modules/search.spec.ts index 7cf4ac35..64f781e6 100644 --- a/src/modules/search.spec.ts +++ b/src/modules/search.spec.ts @@ -43,13 +43,4 @@ describe('search', () => { yf._opts.validation.logErrors = true; }); - it('does not throw on unexpected input if called with {validateResult: false}', async () => { - yf._opts.validation.logErrors = false; - await expect(yf.search('AAPL', {}, { - devel: 'search-fakeBadResult.json', - validateResult: false - })).resolves.toBeDefined(); - yf._opts.validation.logErrors = true; - }); - });