fix: Fix issue when there are multiple values for the same CLI parameter

* fix: added check for multiple values for the same option * Update test/unit/init/CliRunner.test.ts Co-authored-by: Ruben Verborgh <[email protected]> * fix: made CliRunner.run sync Co-authored-by: Ruben Verborgh <[email protected]>
CommunitySolidServer · Mar 5, 2021 · dd5b496 · dd5b496
1 parent 12ace1b
commit dd5b496
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 11 deletions.
diff --git a/src/init/CliRunner.ts b/src/init/CliRunner.ts
@@ -13,18 +13,19 @@ export class CliRunner {
 
   /**
    * Generic run function for starting the server from a given config
+   * Made run to be non-async to lower the chance of unhandled promise rejection errors in the future.
    * @param args - Command line arguments.
    * @param stderr - Standard error stream.
    */
-  public async run({
+  public run({
     argv = process.argv,
     stderr = process.stderr,
   }: {
     argv?: string[];
     stdin?: ReadStream;
     stdout?: WriteStream;
     stderr?: WriteStream;
-  } = {}): Promise<void> {
+  } = {}): void {
     // Parse the command-line arguments
     const { argv: params } = yargs(argv.slice(2))
       .usage('node ./bin/server.js [args]')
@@ -44,6 +45,10 @@ export class CliRunner {
             if (!args[key]) {
               throw new Error(`Missing value for argument "${key}"`);
             }
+            // Check if the argument only has 1 value
+            if (Array.isArray(args[key])) {
+              throw new Error(`Multiple values were provided for: "${key}", [${args[key]}]`);
+            }
           }
         }
         return true;
@@ -71,7 +76,7 @@ export class CliRunner {
     const variables = this.createVariables(params);
 
     // Create and execute the server initializer
-    await this.createInitializer(loaderProperties, configFile, variables)
+    this.createInitializer(loaderProperties, configFile, variables)
       .then(
         async(initializer): Promise<void> => initializer.handleSafe(),
         (error: Error): void => {

diff --git a/test/unit/init/CliRunner.test.ts b/test/unit/init/CliRunner.test.ts
@@ -31,10 +31,15 @@ describe('CliRunner', (): void => {
   });
 
   it('starts the server with default settings.', async(): Promise<void> => {
-    await new CliRunner().run({
+    new CliRunner().run({
       argv: [ 'node', 'script' ],
     });
 
+    // Wait until initializer has been called, because we can't await CliRunner.run.
+    await new Promise((resolve): void => {
+      setImmediate(resolve);
+    });
+
     expect(ComponentsManager.build).toHaveBeenCalledTimes(1);
     expect(ComponentsManager.build).toHaveBeenCalledWith({
       dumpErrorState: true,
@@ -64,7 +69,7 @@ describe('CliRunner', (): void => {
   });
 
   it('accepts abbreviated flags.', async(): Promise<void> => {
-    await new CliRunner().run({
+    new CliRunner().run({
       argv: [
         'node', 'script',
         '-b', 'http://pod.example/',
@@ -79,6 +84,11 @@ describe('CliRunner', (): void => {
       ],
     });
 
+    // Wait until initializer has been called, because we can't await CliRunner.run.
+    await new Promise((resolve): void => {
+      setImmediate(resolve);
+    });
+
     expect(ComponentsManager.build).toHaveBeenCalledTimes(1);
     expect(ComponentsManager.build).toHaveBeenCalledWith({
       dumpErrorState: true,
@@ -105,7 +115,7 @@ describe('CliRunner', (): void => {
   });
 
   it('accepts full flags.', async(): Promise<void> => {
-    await new CliRunner().run({
+    new CliRunner().run({
       argv: [
         'node', 'script',
         '--baseUrl', 'http://pod.example/',
@@ -120,6 +130,11 @@ describe('CliRunner', (): void => {
       ],
     });
 
+    // Wait until initializer has been called, because we can't await CliRunner.run.
+    await new Promise((resolve): void => {
+      setImmediate(resolve);
+    });
+
     expect(ComponentsManager.build).toHaveBeenCalledTimes(1);
     expect(ComponentsManager.build).toHaveBeenCalledWith({
       dumpErrorState: true,
@@ -147,10 +162,15 @@ describe('CliRunner', (): void => {
 
   it('exits with output to stderr when instantiation fails.', async(): Promise<void> => {
     manager.instantiate.mockRejectedValueOnce(new Error('Fatal'));
-    await new CliRunner().run({
+    new CliRunner().run({
       argv: [ 'node', 'script' ],
     });
 
+    // Wait until initializer has been called, because we can't await CliRunner.run.
+    await new Promise((resolve): void => {
+      setImmediate(resolve);
+    });
+
     expect(write).toHaveBeenCalledTimes(2);
     expect(write).toHaveBeenNthCalledWith(1,
       expect.stringMatching(/^Error: could not instantiate server from .*config-default\.json/u));
@@ -163,42 +183,78 @@ describe('CliRunner', (): void => {
 
   it('exits without output to stderr when initialization fails.', async(): Promise<void> => {
     initializer.handleSafe.mockRejectedValueOnce(new Error('Fatal'));
-    await new CliRunner().run();
+    new CliRunner().run();
+
+    // Wait until initializer has been called, because we can't await CliRunner.run.
+    await new Promise((resolve): void => {
+      setImmediate(resolve);
+    });
 
     expect(write).toHaveBeenCalledTimes(0);
 
     expect(exit).toHaveBeenCalledWith(1);
   });
 
   it('exits when unknown options are passed to the main executable.', async(): Promise<void> => {
-    await new CliRunner().run({
+    new CliRunner().run({
       argv: [
         'node', 'script', '--foo',
       ],
     });
 
+    // Wait until initializer has been called, because we can't await CliRunner.run.
+    await new Promise((resolve): void => {
+      setImmediate(resolve);
+    });
+
     expect(exit).toHaveBeenCalledTimes(1);
     expect(exit).toHaveBeenCalledWith(1);
   });
 
   it('exits when no value is passed to the main executable for an argument.', async(): Promise<void> => {
-    await new CliRunner().run({
+    new CliRunner().run({
       argv: [
         'node', 'script', '-s',
       ],
     });
 
+    // Wait until initializer has been called, because we can't await CliRunner.run.
+    await new Promise((resolve): void => {
+      setImmediate(resolve);
+    });
+
     expect(exit).toHaveBeenCalledTimes(1);
     expect(exit).toHaveBeenCalledWith(1);
   });
 
   it('exits when unknown parameters are passed to the main executable.', async(): Promise<void> => {
-    await new CliRunner().run({
+    new CliRunner().run({
       argv: [
         'node', 'script', 'foo', 'bar', 'foo.txt', 'bar.txt',
       ],
     });
 
+    // Wait until initializer has been called, because we can't await CliRunner.run.
+    await new Promise((resolve): void => {
+      setImmediate(resolve);
+    });
+
+    expect(exit).toHaveBeenCalledTimes(1);
+    expect(exit).toHaveBeenCalledWith(1);
+  });
+
+  it('exits when multiple values for a parameter are passed.', async(): Promise<void> => {
+    new CliRunner().run({
+      argv: [
+        'node', 'script', '-ll',
+      ],
+    });
+
+    // Wait until initializer has been called, because we can't await CliRunner.run.
+    await new Promise((resolve): void => {
+      setImmediate(resolve);
+    });
+
     expect(exit).toHaveBeenCalledTimes(1);
     expect(exit).toHaveBeenCalledWith(1);
   });