Skip to content

Commit

Permalink
Merge pull request #1251 from FoalTS/script-errors
Browse files Browse the repository at this point in the history
[CLI] Improve logging of script arguments validation errors
  • Loading branch information
LoicPoullain authored Mar 6, 2024
2 parents 295346e + 5f9f0ad commit 7edd718
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,4 @@ foal run create-todo text="Write tests"

> Note that if you try to create a new to-do without specifying the text argument, you'll get the error below.
>
> `Error: The command line arguments should have required property 'text'.`
> `Script error: arguments must have required property 'text'.`
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,4 @@ foal run create-todo text="Write tests"

> Observe que si intenta crear una nueva tarea sin especificar el argumento del texto, obtendrá el siguiente error.
>
> `Error: The command line arguments should have required property 'text'.`
> `Script error: arguments must have required property 'text'.`
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,4 @@ foal run create-todo text="Write tests"

> Notez que si vous essayez de créer une nouvelle tâche sans spécifier l'argument texte, vous obtiendrez l'erreur ci-dessous.
>
> `Error: The command line arguments should have required property 'text'.`
> `Script error: arguments must have required property 'text'.`
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,4 @@ foal run create-todo text="Write tests"

> Note that if you try to create a new to-do without specifying the text argument, you'll get the error below.
>
> `Error: The command line arguments should have required property 'text'.`
> `Script error: arguments must have required property 'text'.`
18 changes: 12 additions & 6 deletions packages/cli/src/run-script/run-script.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,14 @@ describe('runScript', () => {
it('should validate the process arguments with the schema if it is given.', async () => {
mkdirIfDoesNotExist('build/scripts');
const scriptContent = `const { writeFileSync } = require('fs');
module.exports.schema = { type: 'object', properties: { email: { type: 'string', format: 'email' } } };
module.exports.schema = { type: 'object', properties: { email: { type: 'string', format: 'email', maxLength: 2 }, password: { type: 'string' }, n: { type: 'number', maximum: 10 } }, required: ['password'] };
module.exports.main = function main(args) {
writeFileSync('my-script-temp', JSON.stringify(args), 'utf8');
}`;
writeFileSync('build/scripts/my-script.js', scriptContent, 'utf8');

let msg;
const log = (message: any) => msg = message;
const msgs: string[] = [];
const log = (message: any) => msgs.push(message);

delete require.cache[join(process.cwd(), `./build/scripts/my-script.js`)];

Expand All @@ -84,11 +84,17 @@ describe('runScript', () => {
'run-script',
'my-script',
'email=bar',
'n=11'
], log);

strictEqual(
msg,
'Error: The command line arguments must match format "email".'
deepStrictEqual(
msgs,
[
'Script error: arguments must have required property \'password\'.',
'Script error: the value of "email" must NOT have more than 2 characters.',
'Script error: the value of "email" must match format "email".',
'Script error: the value of "n" must be <= 10.',
]
);
});

Expand Down
8 changes: 6 additions & 2 deletions packages/cli/src/run-script/run-script.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,15 @@ export async function runScript({ name }: { name: string }, argv: string[], log
const args = getCommandLineArguments(argv);

if (schema) {
const ajv = new Ajv({ useDefaults: true });
const ajv = new Ajv({ useDefaults: true, allErrors: true });
addFormats(ajv);
if (!ajv.validate(schema, args)) {
ajv.errors!.forEach(err => {
log(`Error: The command line arguments ${err.message}.`);
if (err.instancePath) {
log(`Script error: the value of "${err.instancePath.split('/')[1]}" ${err.message}.`);
} else {
log(`Script error: arguments ${err.message}.`);
}
});
return;
}
Expand Down

0 comments on commit 7edd718

Please sign in to comment.