diff --git a/.changeset/wild-rabbits-thank.md b/.changeset/wild-rabbits-thank.md new file mode 100644 index 00000000..e0c8a6fc --- /dev/null +++ b/.changeset/wild-rabbits-thank.md @@ -0,0 +1,5 @@ +--- +"@nodesecure/scanner": minor +--- + +feat(scanner): improve error handling for depwalker diff --git a/workspaces/scanner/src/class/StatsCollector.class.ts b/workspaces/scanner/src/class/StatsCollector.class.ts index 194ce60d..efc316ac 100644 --- a/workspaces/scanner/src/class/StatsCollector.class.ts +++ b/workspaces/scanner/src/class/StatsCollector.class.ts @@ -1,11 +1,15 @@ +// Import Third-party Dependencies +import { isHTTPError } from "@openally/httpie"; + // Import Internal Dependencies import { SystemDateProvider, type DateProvider } from "./DateProvider.class.ts"; -import type { ApiStats, Stats } from "../types.ts"; +import type { ApiStats, Stats, Error } from "../types.ts"; export class StatsCollector { #apiCalls: ApiStats[] = []; #dateProvider: DateProvider; #startedAt: number; + #errors: Error[] = []; constructor(dateProvider: DateProvider = new SystemDateProvider()) { this.#dateProvider = dateProvider; @@ -17,8 +21,13 @@ export class StatsCollector { try { const result = fn(); if (result instanceof Promise) { - return result.finally(() => this.#addApiStat(name, startedAt) - ) as ReturnType; + return result + .catch((err) => { + this.#addError(name, err); + throw err; + }) + .finally(() => this.#addApiStat(name, startedAt) + ) as ReturnType; } this.#addApiStat(name, startedAt); @@ -27,6 +36,7 @@ export class StatsCollector { } catch (err) { this.#addApiStat(name, startedAt); + this.#addError(name, err); throw err; } } @@ -39,12 +49,28 @@ export class StatsCollector { }); } + #addError(name: string, err: unknown) { + const error: Error = { + name + }; + if (err instanceof Error) { + error.message = err.message; + error.stack = err.stack; + } + if (isHTTPError(err)) { + error.statusCode = err.statusCode; + } + this.#errors.push(error); + } + getStats(): Stats { return { startedAt: this.#startedAt, executionTime: this.#dateProvider.now() - this.#startedAt, apiCalls: this.#apiCalls, - apiCallsCount: this.#apiCalls.length + apiCallsCount: this.#apiCalls.length, + errorCount: this.#errors.length, + errors: this.#errors }; } } diff --git a/workspaces/scanner/src/types.ts b/workspaces/scanner/src/types.ts index 2f48a41e..01fd49dc 100644 --- a/workspaces/scanner/src/types.ts +++ b/workspaces/scanner/src/types.ts @@ -200,6 +200,16 @@ export type ApiStats = { name: string; }; +export type Error = { + name: string; + message?: string; + stack?: string; + /** + * HTTP Status code + */ + statusCode?: number; +}; + export type Stats = { /** * UNIX Timestamp when the scan started @@ -216,6 +226,12 @@ export type Stats = { apiCallsCount: number; apiCalls: ApiStats[]; + /** + * Number of errors + */ + errorCount: number; + + errors: Error[]; }; export type Identifier = { diff --git a/workspaces/scanner/test/StatsCollector.spec.ts b/workspaces/scanner/test/StatsCollector.spec.ts index c8597c11..2126cfa7 100644 --- a/workspaces/scanner/test/StatsCollector.spec.ts +++ b/workspaces/scanner/test/StatsCollector.spec.ts @@ -2,86 +2,212 @@ import { describe, it } from "node:test"; import assert from "node:assert"; +// Import Third-party Dependencies +import * as npmRegistrySDK from "@nodesecure/npm-registry-sdk"; + // Import Internal Dependencies import type { DateProvider } from "../src/class/DateProvider.class.ts"; import { StatsCollector } from "../src/class/StatsCollector.class.ts"; describe("StatsCollectors", () => { - it("should get the expected global start and execution time", () => { - const dateProvider = new FakeDateProvider(); - dateProvider.setNow(1658512000000); - const statsCollector = new StatsCollector(dateProvider); - dateProvider.setNow(1658512001000); - const { startedAt, executionTime } = statsCollector.getStats(); - assert.strictEqual(startedAt, 1658512000000); - assert.strictEqual(executionTime, 1000); - }); + describe("api calls", () => { + it("should get the expected global start and execution time", () => { + const dateProvider = new FakeDateProvider(); + dateProvider.setNow(1658512000000); + const statsCollector = new StatsCollector(dateProvider); + dateProvider.setNow(1658512001000); + const { startedAt, executionTime } = statsCollector.getStats(); + assert.strictEqual(startedAt, 1658512000000); + assert.strictEqual(executionTime, 1000); + }); - it("should still record the exexution time if the function being tracked throws", () => { - const dateProvider = new FakeDateProvider(); - dateProvider.setNow(1658512000000); - const statsCollector = new StatsCollector(dateProvider); - assert.throws(() => { - statsCollector.track("api/test/1", () => { - dateProvider.setNow(1658512001000); - throw new Error("oh no!"); + it("should still record the exexution time if the function being tracked throws", () => { + const dateProvider = new FakeDateProvider(); + dateProvider.setNow(1658512000000); + const statsCollector = new StatsCollector(dateProvider); + assert.throws(() => { + statsCollector.track("api/test/1", () => { + dateProvider.setNow(1658512001000); + throw new Error("oh no!"); + }); }); + + const { apiCalls, apiCallsCount } = statsCollector.getStats(); + assert.strictEqual(apiCallsCount, 1); + assert.deepEqual(apiCalls, [ + { + name: "api/test/1", + startedAt: 1658512000000, + executionTime: 1000 + } + + ]); }); - const { apiCalls, apiCallsCount } = statsCollector.getStats(); - assert.strictEqual(apiCallsCount, 1); - assert.deepEqual(apiCalls, [ - { - name: "api/test/1", - startedAt: 1658512000000, - executionTime: 1000 - } + it("should be able to track the start and execution time of external api call", async() => { + let hasFnOneBeenCalled = false; + let hasFnTwoBeenCalled = false; + const dateProvider = new FakeDateProvider(); + dateProvider.setNow(1658512000000); + const statsCollector = new StatsCollector(dateProvider); + dateProvider.setNow(1658512001001); + const promise = statsCollector.track("api/test/1", () => { + hasFnOneBeenCalled = true; + + return Promise.resolve(1); + }); + + dateProvider.setNow(1658512002000); + const promiseResult = await promise; - ]); + dateProvider.setNow(1658512003000); + const fnResult = statsCollector.track("api/test/2", () => { + hasFnTwoBeenCalled = true; + dateProvider.setNow(1658512004000); + + return null; + }); + dateProvider.setNow(1658512005000); + const { apiCalls, apiCallsCount } = statsCollector.getStats(); + assert.strictEqual(promiseResult, 1); + assert.strictEqual(fnResult, null); + assert.strictEqual(hasFnOneBeenCalled, true); + assert.strictEqual(hasFnTwoBeenCalled, true); + assert.strictEqual(apiCallsCount, 2); + assert.deepEqual(apiCalls, [ + { + name: "api/test/1", + startedAt: 1658512001001, + executionTime: 999 + }, + { + name: "api/test/2", + startedAt: 1658512003000, + executionTime: 1000 + } + ]); + }); }); - it("should be able to track the start and execution time of external api call", async() => { - let hasFnOneBeenCalled = false; - let hasFnTwoBeenCalled = false; - const dateProvider = new FakeDateProvider(); - dateProvider.setNow(1658512000000); - const statsCollector = new StatsCollector(dateProvider); - dateProvider.setNow(1658512001001); - const promise = statsCollector.track("api/test/1", () => { - hasFnOneBeenCalled = true; - - return Promise.resolve(1); + describe("errors", () => { + it("should have no errors when no tracked function throwed", () => { + const dateProvider = new FakeDateProvider(); + const statsCollector = new StatsCollector(dateProvider); + const { errors, errorCount } = statsCollector.getStats(); + assert.strictEqual(errorCount, 0); + assert.strictEqual(errors.length, 0); + }); + + it("should record when a sync error occurs", () => { + const dateProvider = new FakeDateProvider(); + dateProvider.setNow(1658512000000); + const statsCollector = new StatsCollector(dateProvider); + assert.throws(() => { + statsCollector.track("api/test/1", () => { + dateProvider.setNow(1658512001000); + throw new Error("oh no!"); + }); + }); + const { errors, errorCount } = statsCollector.getStats(); + assert.strictEqual(errorCount, 1); + assert.strictEqual(errors.length, 1); + assert.partialDeepStrictEqual(errors, [{ + name: "api/test/1", + message: "oh no!" + }]); + }); + + it("should record when an error that is not an instance of error occurs", () => { + const dateProvider = new FakeDateProvider(); + dateProvider.setNow(1658512000000); + const statsCollector = new StatsCollector(dateProvider); + assert.throws(() => { + statsCollector.track("api/test/1", () => { + dateProvider.setNow(1658512001000); + // eslint-disable-next-line no-throw-literal + throw null; + }); + }); + const { errors, errorCount } = statsCollector.getStats(); + assert.strictEqual(errorCount, 1); + assert.strictEqual(errors.length, 1); + assert.partialDeepStrictEqual(errors, [{ + name: "api/test/1" + }]); }); - dateProvider.setNow(1658512002000); - const promiseResult = await promise; + it("should have no errors when no async tracked function rejected", async() => { + const dateProvider = new FakeDateProvider(); + dateProvider.setNow(1658512000000); + const statsCollector = new StatsCollector(dateProvider); + await statsCollector.track("api/test/1", async() => { + dateProvider.setNow(1658512001000); + + return Promise.resolve(42); + }); + const { errors, errorCount } = statsCollector.getStats(); + assert.strictEqual(errorCount, 0); + assert.strictEqual(errors.length, 0); + }); - dateProvider.setNow(1658512003000); - const fnResult = statsCollector.track("api/test/2", () => { - hasFnTwoBeenCalled = true; - dateProvider.setNow(1658512004000); + it("should record when an async error occurs", async() => { + const dateProvider = new FakeDateProvider(); + dateProvider.setNow(1658512000000); + const statsCollector = new StatsCollector(dateProvider); + await assert.rejects(async() => { + await statsCollector.track("api/test/1", async() => { + dateProvider.setNow(1658512001000); + throw new Error("async oh no!"); + }); + }); + const { errors, errorCount } = statsCollector.getStats(); + assert.strictEqual(errorCount, 1); + assert.strictEqual(errors.length, 1); + assert.partialDeepStrictEqual(errors, [{ + name: "api/test/1", + message: "async oh no!" + }]); + }); - return null; + it("should record when an async error that is not an instance of error occurs", async() => { + const dateProvider = new FakeDateProvider(); + dateProvider.setNow(1658512000000); + const statsCollector = new StatsCollector(dateProvider); + await assert.rejects(async() => { + await statsCollector.track("api/test/1", async() => { + dateProvider.setNow(1658512001000); + // eslint-disable-next-line no-throw-literal + throw "string error"; + }); + }); + const { errors, errorCount } = statsCollector.getStats(); + assert.strictEqual(errorCount, 1); + assert.strictEqual(errors.length, 1); + assert.partialDeepStrictEqual(errors, [{ + name: "api/test/1" + }]); }); - dateProvider.setNow(1658512005000); - const { apiCalls, apiCallsCount } = statsCollector.getStats(); - assert.strictEqual(promiseResult, 1); - assert.strictEqual(fnResult, null); - assert.strictEqual(hasFnOneBeenCalled, true); - assert.strictEqual(hasFnTwoBeenCalled, true); - assert.strictEqual(apiCallsCount, 2); - assert.deepEqual(apiCalls, [ - { + + it("should add the status code when there is an http error", async() => { + const dateProvider = new FakeDateProvider(); + dateProvider.setNow(1658512000000); + const statsCollector = new StatsCollector(dateProvider); + await assert.rejects(async() => { + await statsCollector.track("api/test/1", async() => { + dateProvider.setNow(1658512001000); + + return npmRegistrySDK.packument("does-not-exist"); + }); + }); + const { errors, errorCount } = statsCollector.getStats(); + assert.strictEqual(errorCount, 1); + assert.strictEqual(errors.length, 1); + assert.partialDeepStrictEqual(errors, [{ name: "api/test/1", - startedAt: 1658512001001, - executionTime: 999 - }, - { - name: "api/test/2", - startedAt: 1658512003000, - executionTime: 1000 - } - ]); + message: "Not Found", + statusCode: 404 + }]); + }); }); }); diff --git a/workspaces/scanner/test/depWalker.spec.ts b/workspaces/scanner/test/depWalker.spec.ts index e2792984..3c8b2dff 100644 --- a/workspaces/scanner/test/depWalker.spec.ts +++ b/workspaces/scanner/test/depWalker.spec.ts @@ -173,6 +173,8 @@ test("execute depWalker on pkg.gitdeps", async(test) => { assert.strictEqual(typeof metadata.executionTime, "number"); assert.strictEqual(Array.isArray(metadata.apiCalls), true); assert.strictEqual(metadata.apiCallsCount, 50); + assert.strictEqual(metadata.errorCount, 2); + assert.strictEqual(metadata.errors.length, 2); }); test("execute depWalker on typo-squatting (with location)", async(test) => {