Skip to content

Commit

Permalink
fix(modules): don't log errors when {validateResult:false} (#16)
Browse files Browse the repository at this point in the history
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 }.
  • Loading branch information
gadicc committed Feb 6, 2021
1 parent b449b4c commit 29f23dc
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 11 deletions.
48 changes: 48 additions & 0 deletions src/lib/moduleExec.spec.ts
Original file line number Diff line number Diff line change
@@ -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();
});

});

});
14 changes: 12 additions & 2 deletions src/lib/moduleExec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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;
}

Expand Down
9 changes: 0 additions & 9 deletions src/modules/search.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});

});

0 comments on commit 29f23dc

Please sign in to comment.