feat: Improve error messages (#108)

netwerk-digitaal-erfgoed · Aug 5, 2024 · 452ed86 · 452ed86
1 parent feeb1d0
commit 452ed86
Show file tree

Hide file tree

Showing 12 changed files with 168 additions and 192 deletions.
diff --git a/jest.config.js b/jest.config.js
@@ -11,10 +11,10 @@ export default {
   coverageReporters: ['json-summary', 'text'],
   coverageThreshold: {
     global: {
-      lines: 71.03,
-      statements: 71.14,
-      branches: 67,
-      functions: 76.76,
+      lines: 71.74,
+      statements: 71.73,
+      branches: 67.17,
+      functions: 80.85,
     },
   },
   transform: {

diff --git a/src/file.ts b/src/file.ts
@@ -14,20 +14,20 @@ import {pipeline as streamPipeline} from 'stream/promises';
 import {Progress} from './progress.js';
 
 export default class File {
-  public readonly $id = 'File';
   private $isValid?: boolean;
 
   public constructor(
     private $path: string,
     private readonly skipExistsCheck: boolean = false
-  ) {}
+  ) {
+    this.validate();
+  }
 
-  public validate(): File {
+  private validate(): File {
     if (this.$isValid !== undefined) return this;
     if (!isFilePathString(this.$path)) {
-      const wrongFilePath: string = this.$path as string;
       throw new Error(
-        `The filename \`${wrongFilePath}\` should start with \`file://\``
+        `The filename \`${this.$path}\` should start with \`file://\``
       );
     }
     this.$path = this.$path.replace(/^file:\/\//, '');
@@ -56,7 +56,6 @@ export default class File {
   }
 
   public toString(): string {
-    this.validate();
     return this.$path;
   }
 

diff --git a/src/generator.ts b/src/generator.ts
@@ -14,7 +14,7 @@ const DEFAULT_BATCH_SIZE = 10;
 
 interface Events {
   end: [iterations: number, statements: number, processed: number];
-  error: [e: Error];
+  error: [e: GeneratorError];
   data: [statement: Quad];
 }
 
@@ -79,31 +79,23 @@ export default class Generator extends EventEmitter<Events> {
   }
 
   private async runBatch(batch: NamedNode[]): Promise<void> {
-    const error = (e: unknown): Error =>
-      new Error(
-        `The Generator did not run successfully, it could not get the results from the endpoint ${
-          this.source?.value
-        }: ${(e as Error).message}`
-      );
-
+    const query = this.query.withIris(batch);
     try {
-      const stream = await this.engine.queryQuads(
-        this.query.withIris(batch).toString(),
-        {
-          sources: [(this.source ??= getEngineSource(this.endpoint))],
-        }
-      );
+      const stream = await this.engine.queryQuads(query.toString(), {
+        sources: [(this.source ??= getEngineSource(this.endpoint))],
+      });
 
       stream.on('data', (quad: Quad) => {
         this.statements++;
         this.emit('data', quad);
       });
-      stream.on('error', e => {
-        this.emit('error', error(e));
-        // reject(e);
+      stream.on('error', error => {
+        this.emit(
+          'error',
+          new GeneratorError({endpoint: this.endpoint, error, query})
+        );
       });
       stream.on('end', () => {
-        // resolve();
         this.iterationsProcessed += batch.length;
         this.emit(
           'end',
@@ -112,8 +104,15 @@ export default class Generator extends EventEmitter<Events> {
           this.iterationsProcessed
         );
       });
-    } catch (e) {
-      this.emit('error', error(e));
+    } catch (error) {
+      this.emit(
+        'error',
+        new GeneratorError({
+          endpoint: this.endpoint,
+          error: error as Error,
+          query,
+        })
+      );
     }
   }
 
@@ -182,3 +181,17 @@ export class Query extends BaseQuery {
     }
   }
 }
+
+export class GeneratorError extends Error {
+  public readonly context;
+  constructor(args: {error: Error; endpoint: Endpoint; query: Query}) {
+    super(
+      `Generator failed for endpoint ${args.endpoint.toString()} with query:\n${args.query.toString()}`,
+      {cause: args.error}
+    );
+    this.context = {
+      endpoint: args.endpoint.toString(),
+      query: args.query.toString(),
+    };
+  }
+}
diff --git a/src/iterator.ts b/src/iterator.ts
@@ -12,7 +12,7 @@ const DEFAULT_LIMIT = 10;
 interface Events {
   data: [$this: NamedNode];
   end: [numResults: number];
-  error: [Error];
+  error: [IteratorError];
 }
 
 export default class Iterator extends EventEmitter<Events> {
@@ -34,14 +34,6 @@ export default class Iterator extends EventEmitter<Events> {
     setTimeout(async () => {
       let resultsPerPage = 0;
       this.query.offset = this.offset;
-      const error = (e: unknown): Error =>
-        new Error(
-          `The Iterator did not run successfully, it could not get the results from the endpoint ${
-            this.source?.value
-          } (offset: ${this.offset}, limit ${this.query.limit}): ${
-            (e as Error).message
-          }`
-        );
       try {
         const stream = await this.engine.queryBindings(this.query.toString(), {
           sources: [(this.source ??= getEngineSource(this.endpoint))],
@@ -65,9 +57,11 @@ export default class Iterator extends EventEmitter<Events> {
           if (this.totalResults === 0) {
             this.emit(
               'error',
-              error(
-                new Error(`no results for query:\n ${this.query.toString()}`)
-              )
+              new IteratorError({
+                endpoint: this.endpoint,
+                query: this.query,
+                error: new Error('No results'),
+              })
             );
           }
           this.offset += this.query.limit;
@@ -79,10 +73,24 @@ export default class Iterator extends EventEmitter<Events> {
         });
 
         stream.on('error', e => {
-          this.emit('error', error(e));
+          this.emit(
+            'error',
+            new IteratorError({
+              error: e,
+              endpoint: this.endpoint,
+              query: this.query,
+            })
+          );
         });
       } catch (e) {
-        this.emit('error', error(e));
+        this.emit(
+          'error',
+          new IteratorError({
+            error: e as Error,
+            endpoint: this.endpoint,
+            query: this.query,
+          })
+        );
       }
     }, this.delay);
   }
@@ -121,3 +129,18 @@ export class Query extends BaseQuery {
     }
   }
 }
+
+export class IteratorError extends Error {
+  public readonly context;
+
+  constructor(args: {error: Error; endpoint: Endpoint; query: Query}) {
+    super(
+      `Iterator failed: ${args.error.message} for endpoint ${args.endpoint.toString()} with query:\n${args.query.toString()}`,
+      {cause: args.error}
+    );
+    this.context = {
+      endpoint: args.endpoint.toString(),
+      query: args.query.toString(),
+    };
+  }
+}
diff --git a/src/main.ts b/src/main.ts
@@ -7,6 +7,7 @@ import type {Configuration} from './configuration.js';
 import loadPipelines from './utils/loadPipelines.js';
 import Pipeline from './pipeline.js';
 import {cli} from './cli.js';
+import Process from 'node:process';
 
 console.info(
   chalk.bold(`Welcome to LD Workbench version ${chalk.cyan(version())}`)
@@ -21,8 +22,7 @@ console.info(
     configuration = pipelines.get(cli.pipeline);
     if (configuration === undefined) {
       error(
-        `No pipeline named “${cli.pipeline}” was found.`,
-        `Valid pipeline names are: ${names.map(name => `"${name}"`).join(', ')}`
+        `No pipeline named “${cli.pipeline}” was found. Valid pipeline names are: ${names.map(name => `"${name}"`).join(', ')}`
       );
     }
   } else if (names.length === 1) {
@@ -61,6 +61,7 @@ console.info(
     });
     await pipeline.run();
   } catch (e) {
-    error(`Error in pipeline ${chalk.italic(configuration.name)}`, e as Error);
+    error(e as Error);
+    Process.exit(1);
   }
 })();
diff --git a/src/pipeline.ts b/src/pipeline.ts
@@ -62,7 +62,7 @@ class Pipeline {
     }
     this.destination = isTriplyDBPathString(destinationFile)
       ? new TriplyDB(destinationFile).validate()
-      : new File(destinationFile, true).validate();
+      : new File(destinationFile, true);
 
     this.stores = (configuration.stores ?? []).map(
       storeConfig =>
@@ -125,9 +125,10 @@ class Pipeline {
         );
       } catch (e) {
         throw new Error(
-          `Error in the iterator of stage \`${stageConfiguration.name}\`: ${
+          `Error in the configuration of stage “${stageConfiguration.name}”: ${
             (e as Error).message
-          }`
+          }`,
+          {cause: e}
         );
       }
     }
@@ -207,7 +208,7 @@ class Pipeline {
         this.increaseProgress(progress, stage, iterationsProcessed, count);
       });
       stage.on('error', e => {
-        progress.fail('Error: ' + (e as Error).message);
+        progress.fail(`Stage “${this.name}” failed`);
         reject(e);
       });
       stage.on('end', (iris, statements) => {

diff --git a/src/stage.ts b/src/stage.ts
@@ -75,7 +75,8 @@ export default class Stage extends EventEmitter<Events> {
         );
       }
     }
-    this.destination = () => new File(this.destinationPath).getStream();
+    this.destination = () =>
+      new File(`file://${this.destinationPath}`, true).getStream();
   }
 
   public get destinationPath(): string {
@@ -157,7 +158,6 @@ export default class Stage extends EventEmitter<Events> {
       this.emit('error', e);
     });
 
-    // Start the iterator
     await this.iterator.run();
   }
 

diff --git a/src/utils/error.ts b/src/utils/error.ts
@@ -1,11 +1,10 @@
-import chalk from 'chalk';
-
-export const error = (
-  message: string | Error,
-  ...info: Array<string | Error>
-): void => {
-  console.error(
-    chalk.red(message instanceof Error ? message.message : message)
-  );
-  info.forEach(i => console.info(i));
+export const error = (message: string | Error): void => {
+  if (message instanceof Error) {
+    console.error(`\n${message.message}`);
+    if (message.cause) {
+      console.error('\nCaused by:\n', message.cause);
+    }
+  } else {
+    console.error(`\n${message}`);
+  }
 };
diff --git a/test/file.test.ts b/test/file.test.ts
@@ -16,39 +16,33 @@ describe('File Class', () => {
       expect(file).to.be.an.instanceOf(File);
       expect(file).to.have.property('$path');
       expect(file).to.have.property('skipExistsCheck');
-      expect(file).to.have.property('$id');
     });
   });
   describe('validate', () => {
     it('should validate a valid file path', () => {
       const path = './static/example/config.yml';
       const validFilePath = `file://${path}`;
       const file = new File(validFilePath);
-      expect(file.validate());
       expect(file.path).to.equal(path);
     });
 
     it('should throw an error for an invalid file path', () => {
       const filePath = 'invalid/file/path.txt';
-      const file = new File(filePath);
-      expect(file.validate.bind(file)).to.throw(
+      expect(() => new File(filePath)).to.throw(
         'The filename `invalid/file/path.txt` should start with `file://`'
       );
     });
 
     it('should throw an error if file does not exist', () => {
       const filePath = 'file://nonexistent/file.txt';
-      const file = new File(filePath);
-      expect(file.validate.bind(file)).to.throw(
+      expect(() => new File(filePath)).to.throw(
         'File not found: `nonexistent/file.txt`'
       );
     });
 
     it('should skip exists check when skipExistsCheck is true', () => {
       const filePath = 'file://nonexistent/file.txt';
-      const file = new File(filePath, true);
-      expect(() => file.validate()).to.not.throw();
-      expect(file.path).to.equal('nonexistent/file.txt');
+      expect(new File(filePath, true).path).to.equal('nonexistent/file.txt');
     });
   });
 
@@ -73,23 +67,23 @@ describe('File Class', () => {
       }
     });
     it('should create a write stream for a new file', () => {
-      const filePath = 'new/file.txt';
-      const file = new File(filePath);
+      const filePath = 'file://new/file.txt';
+      const file = new File(filePath, true);
       writeStream = file.getStream();
       expect(writeStream).to.be.an.instanceOf(fs.WriteStream);
       writeStream.end();
     });
 
     it('should append to an existing file when append is true', () => {
-      const filePath = 'file.txt';
+      const filePath = 'file://file.txt';
       const file = new File(filePath);
       writeStream = file.getStream(true);
       expect(writeStream).to.be.an.instanceOf(fs.WriteStream);
     });
 
     it('should create parent directories if they do not exist', async () => {
       const filePath = 'file://new/directory/nested/file.txt';
-      const file = new File(filePath, true).validate();
+      const file = new File(filePath, true);
       writeStream = file.getStream();
       expect(writeStream).to.be.an.instanceOf(fs.WriteStream);
       expect(