diff --git a/packages/@nitpicker/analyze-lighthouse/src/index.spec.ts b/packages/@nitpicker/analyze-lighthouse/src/index.spec.ts index 2d3653f..be65d33 100644 --- a/packages/@nitpicker/analyze-lighthouse/src/index.spec.ts +++ b/packages/@nitpicker/analyze-lighthouse/src/index.spec.ts @@ -120,17 +120,28 @@ describe('analyze-lighthouse plugin', () => { }); }); - it('propagates non-Error exceptions without swallowing them', async () => { + it('wraps non-Error exceptions into Error and returns error result', async () => { lighthouseMock.mockRejectedValue('unexpected string throw'); const plugin = pluginFactory({}, ''); const url = new URL('https://example.com'); - - await expect( - plugin.eachPage!({ url, html: '', window: {} as never, num: 0, total: 1 }), - ).rejects.toBe('unexpected string throw'); + const result = await plugin.eachPage!({ + url, + html: '', + window: {} as never, + num: 0, + total: 1, + }); expect(killMock).toHaveBeenCalledTimes(1); + expect(result).toEqual({ + page: { + performance: { value: 0, note: 'Error' }, + accessibility: { value: 0, note: 'Error' }, + 'best-practices': { value: 0, note: 'Error' }, + seo: { value: 0, note: 'Error' }, + }, + }); }); it('propagates unexpected errors from report processing without swallowing them', async () => { diff --git a/packages/@nitpicker/analyze-lighthouse/src/index.ts b/packages/@nitpicker/analyze-lighthouse/src/index.ts index ebb7100..4dd6f0b 100644 --- a/packages/@nitpicker/analyze-lighthouse/src/index.ts +++ b/packages/@nitpicker/analyze-lighthouse/src/index.ts @@ -6,6 +6,8 @@ import * as chromeLauncher from 'chrome-launcher'; import lighthouse from 'lighthouse'; import { ReportUtils } from 'lighthouse/report/renderer/report-utils.js'; +import { toError } from './to-error.js'; + /** * Plugin options for the Lighthouse analysis. */ @@ -62,12 +64,7 @@ export default definePlugin((options: Options) => { try { const result = await lighthouse(url.href, { port: chrome.port }, config).catch( - (error: unknown) => { - if (error instanceof Error) { - return error; - } - throw error; - }, + (error: unknown) => toError(error), ); if (!result || result instanceof Error) { diff --git a/packages/@nitpicker/analyze-lighthouse/src/to-error.spec.ts b/packages/@nitpicker/analyze-lighthouse/src/to-error.spec.ts new file mode 100644 index 0000000..4754498 --- /dev/null +++ b/packages/@nitpicker/analyze-lighthouse/src/to-error.spec.ts @@ -0,0 +1,36 @@ +import { describe, it, expect } from 'vitest'; + +import { toError } from './to-error.js'; + +describe('toError', () => { + it('returns the same Error instance when given an Error', () => { + const original = new Error('original message'); + const result = toError(original); + expect(result).toBe(original); + }); + + it('wraps a string into a new Error', () => { + const result = toError('string message'); + expect(result).toBeInstanceOf(Error); + expect(result.message).toBe('string message'); + }); + + it('wraps null into a new Error', () => { + const result = toError(null); + expect(result).toBeInstanceOf(Error); + expect(result.message).toBe('null'); + }); + + it('wraps undefined into a new Error', () => { + const result = toError(); + expect(result).toBeInstanceOf(Error); + expect(result.message).toBe('undefined'); + }); + + it('preserves subclass instances of Error', () => { + const original = new TypeError('type error'); + const result = toError(original); + expect(result).toBe(original); + expect(result).toBeInstanceOf(TypeError); + }); +}); diff --git a/packages/@nitpicker/analyze-lighthouse/src/to-error.ts b/packages/@nitpicker/analyze-lighthouse/src/to-error.ts new file mode 100644 index 0000000..f8e8d3e --- /dev/null +++ b/packages/@nitpicker/analyze-lighthouse/src/to-error.ts @@ -0,0 +1,15 @@ +/** + * Coerces an unknown thrown value into an `Error` instance. + * + * If the value is already an `Error`, it is returned as-is so that its + * original `stack` trace and any custom properties are preserved. + * Otherwise, the value is converted to a string and wrapped in a new `Error`. + * @param value - The caught value to normalise. + * @returns An `Error` instance, either the original or a newly created one. + */ +export function toError(value?: unknown): Error { + if (value instanceof Error) { + return value; + } + return new Error(String(value)); +} diff --git a/packages/@nitpicker/cli/src/commands/crawl.ts b/packages/@nitpicker/cli/src/commands/crawl.ts index 85b876a..1796c45 100644 --- a/packages/@nitpicker/cli/src/commands/crawl.ts +++ b/packages/@nitpicker/cli/src/commands/crawl.ts @@ -169,10 +169,11 @@ function run( * Starts a fresh crawl session for the given URLs. * * Creates a CrawlerOrchestrator, runs the crawl, writes the archive, - * and cleans up browser processes. Exits with code 1 if errors occurred. + * and cleans up browser processes. * @param siteUrl - One or more root URLs to crawl * @param flags - Parsed CLI flags from the `crawl` command * @returns A promise that resolves with the archive file path when crawling, writing, and cleanup are complete. + * @throws {CrawlAggregateError} When one or more errors occurred during crawling. */ export async function startCrawl(siteUrl: string[], flags: CrawlFlags): Promise { const errStack: (CrawlerError | Error)[] = []; @@ -205,7 +206,7 @@ export async function startCrawl(siteUrl: string[], flags: CrawlFlags): Promise< if (errStack.length > 0) { formatCrawlErrors(errStack); - process.exit(1); + throw new CrawlAggregateError(errStack); } return archivePath; @@ -248,7 +249,7 @@ async function resumeCrawl(stubFilePath: string, flags: CrawlFlags) { if (errStack.length > 0) { formatCrawlErrors(errStack); - process.exit(1); + throw new CrawlAggregateError(errStack); } } @@ -281,39 +282,62 @@ export async function crawl(args: string[], flags: CrawlFlags) { return; } - if (flags.resume) { - if (flags.output) { - throw new Error( - '--output flag is not supported with --resume. The archive path is determined by the stub file.', - ); - } - await resumeCrawl(flags.resume, flags); - return; - } - if (flags.single && (flags.list?.length || flags.listFile)) { // eslint-disable-next-line no-console console.warn('Warning: --single is ignored when --list or --list-file is specified.'); } - if (flags.listFile) { - const list = await readList(path.resolve(process.cwd(), flags.listFile)); - flags.list = list; - await startCrawl(list, flags); - return; - } + try { + if (flags.resume) { + if (flags.output) { + throw new Error( + '--output flag is not supported with --resume. The archive path is determined by the stub file.', + ); + } + await resumeCrawl(flags.resume, flags); + return; + } - if (flags.list && flags.list.length > 0) { - const pageList = [...flags.list, ...args]; - await startCrawl(pageList, flags); - return; - } + if (flags.listFile) { + const list = await readList(path.resolve(process.cwd(), flags.listFile)); + flags.list = list; + await startCrawl(list, flags); + return; + } - const siteUrl = args[0]; + if (flags.list && flags.list.length > 0) { + const pageList = [...flags.list, ...args]; + await startCrawl(pageList, flags); + return; + } - if (siteUrl) { - await startCrawl([siteUrl], flags); - return; + const siteUrl = args[0]; + + if (siteUrl) { + await startCrawl([siteUrl], flags); + return; + } + } catch (error) { + if (error instanceof CrawlAggregateError) { + process.exit(1); + } + throw error; + } +} + +/** + * Error thrown when one or more errors occurred during crawling. + * Wraps the collected errors so callers can inspect them. + */ +export class CrawlAggregateError extends Error { + /** The individual errors that occurred during crawling. */ + readonly errors: readonly (CrawlerError | Error)[]; + /** + * @param errors - The individual errors collected during the crawl session. + */ + constructor(errors: (CrawlerError | Error)[]) { + super(`Crawl completed with ${errors.length} error(s).`); + this.errors = errors; } } diff --git a/packages/@nitpicker/cli/src/commands/pipeline.spec.ts b/packages/@nitpicker/cli/src/commands/pipeline.spec.ts index 064aabe..9648673 100644 --- a/packages/@nitpicker/cli/src/commands/pipeline.spec.ts +++ b/packages/@nitpicker/cli/src/commands/pipeline.spec.ts @@ -288,6 +288,33 @@ describe('pipeline command', () => { ).rejects.toThrow('Report failed'); }); + it('suppresses pipeline log output when --silent is set', async () => { + vi.mocked(startCrawlFn).mockResolvedValue('/tmp/site.nitpicker'); + vi.mocked(analyzeFn).mockResolvedValue(); + + await pipeline(['https://example.com'], { + ...defaultFlags, + silent: true, + all: true, + }); + + expect(consoleLogSpy).not.toHaveBeenCalled(); + }); + + it('suppresses pipeline log output when --silent is set with --sheet', async () => { + vi.mocked(startCrawlFn).mockResolvedValue('/tmp/site.nitpicker'); + vi.mocked(analyzeFn).mockResolvedValue(); + vi.mocked(reportFn).mockResolvedValue(); + + await pipeline(['https://example.com'], { + ...defaultFlags, + silent: true, + sheet: 'https://docs.google.com/spreadsheets/d/xxx', + }); + + expect(consoleLogSpy).not.toHaveBeenCalled(); + }); + it('shows completion message after all steps', async () => { vi.mocked(startCrawlFn).mockResolvedValue('/tmp/site.nitpicker'); vi.mocked(analyzeFn).mockResolvedValue(); diff --git a/packages/@nitpicker/cli/src/commands/pipeline.ts b/packages/@nitpicker/cli/src/commands/pipeline.ts index 08532b0..5e8da29 100644 --- a/packages/@nitpicker/cli/src/commands/pipeline.ts +++ b/packages/@nitpicker/cli/src/commands/pipeline.ts @@ -10,6 +10,9 @@ import { report } from './report.js'; * that executes the full crawl → analyze → report workflow sequentially. * @see {@link pipeline} for the main entry point */ +// TODO: フラグ定義が crawl.ts / analyze.ts / report.ts と重複している。 +// @d-zero/roar の CommandDef 型制約により合成が困難なため手動同期が必要。 +// crawl / analyze / report にフラグを追加・変更した際はここも更新すること。 export const commandDef = { desc: 'Run crawl → analyze → report sequentially', flags: { @@ -175,7 +178,7 @@ type PipelineFlags = InferFlags; * to the analyze step. If `--sheet` is provided, the report step runs * last to publish results to Google Sheets. * - * Each step's errors cause `process.exit(1)`, halting the pipeline. + * Errors from any step propagate to the caller as exceptions. * @param args - Positional arguments; first argument is the root URL to crawl. * @param flags - Parsed CLI flags from the `pipeline` command. * @returns Resolves when all pipeline steps complete. @@ -191,9 +194,13 @@ export async function pipeline(args: string[], flags: PipelineFlags) { process.exit(1); } + const silent = !!flags.silent; + // Step 1: Crawl - // eslint-disable-next-line no-console - console.log('\n📡 [pipeline] Step 1/3: Crawling...'); + if (!silent) { + // eslint-disable-next-line no-console + console.log('\n📡 [pipeline] Step 1/3: Crawling...'); + } const archivePath = await startCrawl([siteUrl], { interval: flags.interval, image: flags.image, @@ -221,8 +228,10 @@ export async function pipeline(args: string[], flags: PipelineFlags) { }); // Step 2: Analyze - // eslint-disable-next-line no-console - console.log('\n🔍 [pipeline] Step 2/3: Analyzing...'); + if (!silent) { + // eslint-disable-next-line no-console + console.log('\n🔍 [pipeline] Step 2/3: Analyzing...'); + } await analyze([archivePath], { all: flags.all, plugin: flags.plugin, @@ -236,8 +245,10 @@ export async function pipeline(args: string[], flags: PipelineFlags) { // Step 3: Report (only if --sheet is provided) if (flags.sheet) { - // eslint-disable-next-line no-console - console.log('\n📊 [pipeline] Step 3/3: Reporting...'); + if (!silent) { + // eslint-disable-next-line no-console + console.log('\n📊 [pipeline] Step 3/3: Reporting...'); + } await report([archivePath], { sheet: flags.sheet, credentials: flags.credentials, @@ -247,11 +258,13 @@ export async function pipeline(args: string[], flags: PipelineFlags) { verbose: flags.verbose, silent: flags.silent, }); - } else { + } else if (!silent) { // eslint-disable-next-line no-console console.log('\n📊 [pipeline] Step 3/3: Skipped (no --sheet specified)'); } - // eslint-disable-next-line no-console - console.log('\n✅ [pipeline] All steps completed.'); + if (!silent) { + // eslint-disable-next-line no-console + console.log('\n✅ [pipeline] All steps completed.'); + } } diff --git a/packages/@nitpicker/crawler/src/archive/archive.spec.ts b/packages/@nitpicker/crawler/src/archive/archive.spec.ts index 1aec26f..540eeb9 100644 --- a/packages/@nitpicker/crawler/src/archive/archive.spec.ts +++ b/packages/@nitpicker/crawler/src/archive/archive.spec.ts @@ -1,7 +1,7 @@ import path from 'node:path'; import { tryParseUrl as parseUrl } from '@d-zero/shared/parse-url'; -import { afterAll, describe, expect, it, vi } from 'vitest'; +import { afterAll, describe, expect, it, vi, type MockInstance } from 'vitest'; import Archive from './archive.js'; import { Database } from './database.js'; @@ -42,41 +42,87 @@ describe('setPage', () => { cwd: workingDir, }); - const pageData = { - url: parseUrl('http://localhost/snapshot-fail')!, - redirectPaths: [] as string[], - isExternal: false, - status: 200, - statusText: 'OK', - contentLength: 100, - contentType: 'text/html', - responseHeaders: {}, - meta: { title: 'Snapshot Fail Test' }, - anchorList: [] as never[], - imageList: [] as never[], - html: 'test', - isSkipped: false, - isTarget: true, - }; - - await expect(archive.setPage(pageData)).rejects.toThrow('Disk write failure'); - - expect(mockedOutputText).toHaveBeenCalledTimes(1); - - // HTMLパスがクリアされていることをDB経由で検証 - const dbPath = path.resolve(tmpDirPattern, Archive.SQLITE_DB_FILE_NAME); - const db = await Database.connect({ - type: 'sqlite3', - workingDir: tmpDirPattern, - filename: dbPath, + try { + const pageData = { + url: parseUrl('http://localhost/snapshot-fail')!, + redirectPaths: [] as string[], + isExternal: false, + status: 200, + statusText: 'OK', + contentLength: 100, + contentType: 'text/html', + responseHeaders: {}, + meta: { title: 'Snapshot Fail Test' }, + anchorList: [] as never[], + imageList: [] as never[], + html: 'test', + isSkipped: false, + isTarget: true, + }; + + await expect(archive.setPage(pageData)).rejects.toThrow('Disk write failure'); + + expect(mockedOutputText).toHaveBeenCalledTimes(1); + + // HTMLパスがクリアされていることをDB経由で検証 + const dbPath = path.resolve(tmpDirPattern, Archive.SQLITE_DB_FILE_NAME); + const db = await Database.connect({ + type: 'sqlite3', + workingDir: tmpDirPattern, + filename: dbPath, + }); + try { + const pages = await db.getPages(); + const page = pages.find((p) => p.url === 'http://localhost/snapshot-fail'); + expect(page).toBeDefined(); + expect(page!.html).toBeNull(); + } finally { + await db.destroy(); + } + } finally { + mockedOutputText.mockRestore(); + await archive.close(); + } + }); + + it('clearHtmlPath失敗時も元のスナップショットエラーが伝搬する', async () => { + const fsIndex = await import('./filesystem/index.js'); + const mockedOutputText = vi.mocked(fsIndex.outputText); + mockedOutputText.mockRejectedValueOnce(new Error('Disk full')); + + const clearSpy: MockInstance = vi + .spyOn(Database.prototype, 'clearHtmlPath') + .mockRejectedValueOnce(new Error('DB locked')); + + const archive = await Archive.create({ + filePath: archiveFilePath, + cwd: workingDir, }); - const pages = await db.getPages(); - const page = pages.find((p) => p.url === 'http://localhost/snapshot-fail'); - expect(page).toBeDefined(); - expect(page!.html).toBeNull(); - - await db.destroy(); - mockedOutputText.mockRestore(); - await archive.close(); + + try { + const pageData = { + url: parseUrl('http://localhost/double-fail')!, + redirectPaths: [] as string[], + isExternal: false, + status: 200, + statusText: 'OK', + contentLength: 50, + contentType: 'text/html', + responseHeaders: {}, + meta: { title: 'Double Fail' }, + anchorList: [] as never[], + imageList: [] as never[], + html: '', + isSkipped: false, + isTarget: true, + }; + + // 元のエラー(Disk full)が伝搬する(DB lockedに差し替わらない) + await expect(archive.setPage(pageData)).rejects.toThrow('Disk full'); + } finally { + clearSpy.mockRestore(); + mockedOutputText.mockRestore(); + await archive.close(); + } }); }); diff --git a/packages/@nitpicker/crawler/src/archive/database.ts b/packages/@nitpicker/crawler/src/archive/database.ts index d369ffb..7c517df 100644 --- a/packages/@nitpicker/crawler/src/archive/database.ts +++ b/packages/@nitpicker/crawler/src/archive/database.ts @@ -1021,6 +1021,7 @@ function redirectTable(includeNull = true) { * Safely parses a JSON string, returning a fallback value if parsing fails or the input is not a string. * Logs a warning via {@link dbLog} when invalid JSON is detected, including a truncated preview * of the data and the parse error message. + * @template T The expected type of the parsed JSON value and the fallback. * @param data - The data to parse. Only string values are parsed; other types return the fallback. * @param fallback - The value to return if parsing fails or the result is falsy. * @returns The parsed JSON value, or the fallback. diff --git a/packages/@nitpicker/crawler/src/write-queue.spec.ts b/packages/@nitpicker/crawler/src/write-queue.spec.ts index 9b937dc..4940a3a 100644 --- a/packages/@nitpicker/crawler/src/write-queue.spec.ts +++ b/packages/@nitpicker/crawler/src/write-queue.spec.ts @@ -132,6 +132,13 @@ describe('WriteQueue', () => { expect(result).toBe('recovered'); }); + it('drain() resolves immediately on an empty queue', async () => { + const queue = new WriteQueue(); + expect(queue.pending).toBe(0); + await queue.drain(); + expect(queue.pending).toBe(0); + }); + it('handles high concurrency without interleaving', async () => { const queue = new WriteQueue(); let active = 0; diff --git a/packages/@nitpicker/crawler/src/write-queue.ts b/packages/@nitpicker/crawler/src/write-queue.ts index 3557015..c70474d 100644 --- a/packages/@nitpicker/crawler/src/write-queue.ts +++ b/packages/@nitpicker/crawler/src/write-queue.ts @@ -30,15 +30,16 @@ export class WriteQueue { } /** - * Enqueues an asynchronous operation to run after all previously enqueued - * operations have completed. Operations are guaranteed to execute serially - * in the order they were enqueued. + * Enqueues an operation to run after all previously enqueued operations + * have completed. Operations are guaranteed to execute serially in the + * order they were enqueued. Both async and synchronous (throwing) + * operations are supported. * @template T The resolved type of the operation's promise. - * @param operation - The async function to execute. + * @param operation - The function to execute. May return a `Promise` or a plain value. * @returns A promise that resolves with the operation's result, or rejects * if the operation throws. */ - enqueue(operation: () => Promise): Promise { + enqueue(operation: () => Promise | T): Promise { this.#pending++; return new Promise((resolve, reject) => { this.#chain = this.#chain.then(async () => { diff --git a/packages/test-server/src/__tests__/e2e/options.e2e.ts b/packages/test-server/src/__tests__/e2e/options.e2e.ts index b755924..ec70d0b 100644 --- a/packages/test-server/src/__tests__/e2e/options.e2e.ts +++ b/packages/test-server/src/__tests__/e2e/options.e2e.ts @@ -19,14 +19,13 @@ describe('Crawler options', () => { it('外部リンクがフェッチされずにDBに記録される', async () => { const pages = await result.accessor.getPages('external-page'); const externalPage = pages.find((p) => p.url.hostname === '127.0.0.1'); - if (externalPage) { - // fetchExternal: false の場合、外部ページはフルスクレイプされない - expect( - externalPage.status === null || - externalPage.status === 0 || - externalPage.title === '', - ).toBe(true); - } + expect(externalPage).toBeDefined(); + // fetchExternal: false の場合、外部ページはフルスクレイプされない + expect( + externalPage!.status === null || + externalPage!.status === 0 || + externalPage!.title === '', + ).toBe(true); }); it('内部ページは通常通りスクレイプされる', async () => { diff --git a/packages/test-server/src/__tests__/e2e/scope.e2e.ts b/packages/test-server/src/__tests__/e2e/scope.e2e.ts index 255659d..c68ffdb 100644 --- a/packages/test-server/src/__tests__/e2e/scope.e2e.ts +++ b/packages/test-server/src/__tests__/e2e/scope.e2e.ts @@ -25,9 +25,8 @@ describe('Scope restriction', () => { it('scope外のページはisTarget=falseで記録される', async () => { const pages = await result.accessor.getPages('page'); const docsPage = pages.find((p) => p.url.pathname === '/scope/docs/'); - if (docsPage) { - expect(docsPage.isTarget).toBe(false); - } + expect(docsPage).toBeDefined(); + expect(docsPage!.isTarget).toBe(false); }); }); diff --git a/packages/test-server/src/__tests__/e2e/single-page.e2e.ts b/packages/test-server/src/__tests__/e2e/single-page.e2e.ts index 4f1db73..a4b67e7 100644 --- a/packages/test-server/src/__tests__/e2e/single-page.e2e.ts +++ b/packages/test-server/src/__tests__/e2e/single-page.e2e.ts @@ -29,9 +29,8 @@ describe('Single page scraping', () => { it('非再帰モードで子ページはisTarget=falseで記録される', async () => { const pages = await result.accessor.getPages('page'); const aboutPages = pages.filter((p) => p.url.pathname === '/about'); - if (aboutPages.length > 0) { - // titleOnlyモードでスクレイプされた場合、isTarget=falseであること - expect(aboutPages[0]!.isTarget).toBe(false); - } + expect(aboutPages.length).toBeGreaterThan(0); + // titleOnlyモードでスクレイプされた場合、isTarget=falseであること + expect(aboutPages[0]!.isTarget).toBe(false); }); });