Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,8 @@ sequenceDiagram

CLI->>GS: report(filePath, sheetUrl, credentials, config, limit, all?, silent?)
GS->>GS: authentication(credentials)(OAuth2)
GS->>Archive: getArchive(filePath)
GS->>Archive: getArchive(filePath) → { archive, removeSignalHandlers }
Note over GS: try/finally で cleanup を保証
GS->>GS: loadConfig(configPath)
GS->>Archive: getPluginReports(archive)

Expand All @@ -513,6 +514,7 @@ sequenceDiagram
Note over GS,Sheets: silent=false 時: Lanes で進捗表示 + レート制限カウントダウン
end

GS->>GS: removeSignalHandlers()
GS->>Archive: archive.close()
```

Expand Down
21 changes: 21 additions & 0 deletions packages/@nitpicker/cli/src/commands/crawl.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,27 @@ describe('startCrawl', () => {
expect(result).toBe('/tmp/test.nitpicker');
});

it('完了後にシグナルリスナーが蓄積しない', async () => {
const { startCrawl } = await import('./crawl.js');
const before = process.listenerCount('SIGINT');

await startCrawl(['https://example.com'], createFlags());
await startCrawl(['https://example.com'], createFlags());
await startCrawl(['https://example.com'], createFlags());

expect(process.listenerCount('SIGINT')).toBe(before);
});

it('eventAssignments がエラーでもシグナルリスナーが解除される', async () => {
mockEventAssignments.mockRejectedValueOnce(new Error('scrape failed'));
const { startCrawl } = await import('./crawl.js');
const before = process.listenerCount('SIGINT');

await startCrawl(['https://example.com'], createFlags()).catch(() => {});

expect(process.listenerCount('SIGINT')).toBe(before);
});

it('イベントエラー発生時に CrawlAggregateError をスローする', async () => {
mockEventAssignments.mockRejectedValueOnce(new Error('scrape failed'));

Expand Down
23 changes: 16 additions & 7 deletions packages/@nitpicker/cli/src/commands/crawl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,16 @@ type LogType = 'verbose' | 'normal' | 'silent';
* crawl via {@link CrawlerOrchestrator.abort}, then kill zombie Chromium
* processes and exit. The abort signal propagates through the dealer's
* AbortSignal mechanism so no new workers are launched.
*
* Signal handlers are automatically removed in a `finally` block when
* the event assignment pipeline completes or throws.
* @param trigger - Display label for the crawl (URL or stub file path)
* @param orchestrator - The initialized CrawlerOrchestrator instance
* @param config - The resolved archive configuration
* @param logType - Output verbosity level
* @returns A promise from the event assignment pipeline.
* @returns A promise that resolves when the event assignment pipeline completes.
*/
function run(
async function run(
trigger: string,
orchestrator: CrawlerOrchestrator,
config: Config,
Expand All @@ -160,16 +163,22 @@ function run(
orchestrator.garbageCollect();
process.exit();
};
process.on('SIGINT', killed);
process.on('SIGBREAK', killed);
process.on('SIGHUP', killed);
process.on('SIGABRT', killed);
const signals: NodeJS.Signals[] = ['SIGINT', 'SIGBREAK', 'SIGHUP', 'SIGABRT'];
for (const signal of signals) {
process.on(signal, killed);
}

const head = [
`🐳 ${trigger} (New scraping)`,
...Object.entries(config).map(([key, value]) => ` ${key}: ${value}`),
];
return eventAssignments(orchestrator, head, logType);
try {
return await eventAssignments(orchestrator, head, logType);
} finally {
for (const signal of signals) {
process.removeListener(signal, killed);
}
}
}

/**
Expand Down
71 changes: 71 additions & 0 deletions packages/@nitpicker/report-google-sheets/src/archive.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { afterEach, beforeEach, describe, it, expect, vi } from 'vitest';

const mockOpen = vi.fn();
const mockClose = vi.fn();

vi.mock('@nitpicker/crawler', () => ({
Archive: {
open: mockOpen,
},
}));

vi.mock('./debug.js', () => ({
archiveLog: vi.fn(),
}));

describe('getArchive', () => {
beforeEach(() => {
vi.clearAllMocks();
mockOpen.mockResolvedValue({ close: mockClose });
});

afterEach(() => {
vi.restoreAllMocks();
});

it('archive と removeSignalHandlers を含むオブジェクトを返す', async () => {
const { getArchive } = await import('./archive.js');
const handle = await getArchive('/tmp/test.nitpicker');

expect(handle).toHaveProperty('archive');
expect(handle).toHaveProperty('removeSignalHandlers');
expect(typeof handle.removeSignalHandlers).toBe('function');
});

it('シグナルリスナーを登録する', async () => {
const { getArchive } = await import('./archive.js');
const before = process.listenerCount('SIGINT');

await getArchive('/tmp/test.nitpicker');

expect(process.listenerCount('SIGINT')).toBe(before + 1);
expect(process.listenerCount('SIGHUP')).toBeGreaterThan(0);
expect(process.listenerCount('SIGABRT')).toBeGreaterThan(0);
});

it('removeSignalHandlers を呼ぶとシグナルリスナーが解除される', async () => {
const { getArchive } = await import('./archive.js');
const before = process.listenerCount('SIGINT');

const { removeSignalHandlers } = await getArchive('/tmp/test.nitpicker');
removeSignalHandlers();

expect(process.listenerCount('SIGINT')).toBe(before);
});

it('複数回呼び出してもリスナーが蓄積しない(removeSignalHandlers で解除した場合)', async () => {
const { getArchive } = await import('./archive.js');
const before = process.listenerCount('SIGINT');

const handle1 = await getArchive('/tmp/test1.nitpicker');
handle1.removeSignalHandlers();

const handle2 = await getArchive('/tmp/test2.nitpicker');
handle2.removeSignalHandlers();

const handle3 = await getArchive('/tmp/test3.nitpicker');
handle3.removeSignalHandlers();

expect(process.listenerCount('SIGINT')).toBe(before);
});
});
34 changes: 27 additions & 7 deletions packages/@nitpicker/report-google-sheets/src/archive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,50 @@ import { Archive } from '@nitpicker/crawler';

import { archiveLog } from './debug.js';

/**
* Result of opening an archive with signal handlers registered.
*/
export interface ArchiveHandle {
/** The opened archive instance. */
readonly archive: Archive;
/** Removes the registered signal handlers. Call this when the archive is no longer in use. */
readonly removeSignalHandlers: () => void;
}

/**
* Opens a `.nitpicker` archive file and registers process signal handlers
* to ensure the archive is closed gracefully on unexpected termination.
*
* The caller must invoke `removeSignalHandlers()` when the archive is no
* longer needed to avoid listener accumulation.
* @param filePath - Path to the `.nitpicker` archive file
* @returns The opened `Archive` instance with plugin data loaded
* @returns An {@link ArchiveHandle} containing the archive and a cleanup function.
*/
export async function getArchive(filePath: string) {
export async function getArchive(filePath: string): Promise<ArchiveHandle> {
archiveLog('Open file: %s', filePath);
const archive = await Archive.open({
filePath,
openPluginData: true,
});
archiveLog('File open succeeded');

const signals: NodeJS.Signals[] = ['SIGINT', 'SIGBREAK', 'SIGHUP', 'SIGABRT'];

const close = async () => {
await archive.close();
process.exit();
};

archiveLog('Bind close method to SIGINT, SIGBREAK, SIGHUP, SIGABRT events');
process.on('SIGINT', close);
process.on('SIGBREAK', close);
process.on('SIGHUP', close);
process.on('SIGABRT', close);
for (const signal of signals) {
process.on(signal, close);
}

const removeSignalHandlers = () => {
for (const signal of signals) {
process.removeListener(signal, close);
}
};

return archive;
return { archive, removeSignalHandlers };
}
25 changes: 24 additions & 1 deletion packages/@nitpicker/report-google-sheets/src/report.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,15 @@ vi.mock('@d-zero/google-sheets', () => ({
},
}));

const { mockArchiveClose, mockRemoveSignalHandlers } = vi.hoisted(() => ({
mockArchiveClose: vi.fn(),
mockRemoveSignalHandlers: vi.fn(),
}));

vi.mock('./archive.js', () => ({
getArchive: vi.fn().mockResolvedValue({
close: vi.fn(),
archive: { close: mockArchiveClose },
removeSignalHandlers: mockRemoveSignalHandlers,
}),
}));

Expand Down Expand Up @@ -87,6 +93,23 @@ describe('report', () => {
expect(lanesSpy).toHaveBeenCalled();
});

it('正常完了時に removeSignalHandlers と archive.close を呼び出す', async () => {
await report({ ...baseParams, all: true });

expect(mockRemoveSignalHandlers).toHaveBeenCalledTimes(1);
expect(mockArchiveClose).toHaveBeenCalledTimes(1);
});

it('createSheets が例外をスローしても removeSignalHandlers と archive.close を呼び出す', async () => {
const { createSheets } = await import('./sheets/create-sheets.js');
vi.mocked(createSheets).mockRejectedValueOnce(new Error('sheets error'));

await expect(report({ ...baseParams, all: true })).rejects.toThrow('sheets error');

expect(mockRemoveSignalHandlers).toHaveBeenCalledTimes(1);
expect(mockArchiveClose).toHaveBeenCalledTimes(1);
});

it('passes all 9 sheets to createSheets when all=true', async () => {
const { createSheets } = await import('./sheets/create-sheets.js');

Expand Down
Loading
Loading