Add comprehensive error handling and test coverage for analyze#47
Merged
YusukeHirao merged 4 commits intomainfrom Mar 5, 2026
Merged
Add comprehensive error handling and test coverage for analyze#47YusukeHirao merged 4 commits intomainfrom
YusukeHirao merged 4 commits intomainfrom
Conversation
YusukeHirao
force-pushed
the
claude/fix-analyze-empty-results-7ieOv
branch
from
6d26a62 to
b8ac0b8
Compare
The bounded Promise pool in Nitpicker.analyze() did not catch individual task errors. When runInWorker() rejected (e.g., worker crash, OOM, plugin import failure), the rejection propagated to Promise.race/Promise.all, crashing the entire analyze flow before saving any results to the archive. Changes: - Wrap each page analysis task in try-catch so a single page failure does not abort the entire plugin run - Emit 'error' events for individual page failures with URL context - Emit a warning when a plugin produces no page data at all - Add try-catch in worker.ts to send a graceful 'finish' message with null result when runner() throws, instead of crashing - Add unit tests covering error resilience and empty-result warnings Closes #35 https://claude.ai/code/session_01GGS8WVVNwxG2ueyEdyVUYW
- Add try-catch to Phase 2 (eachUrl) so a single plugin failure does
not abort remaining URL checks or prevent report saving
- Propagate worker error field in run-in-worker.ts: when worker.ts
sends { error }, reject the promise instead of silently resolving null
- Re-throw plugin errors from page-analysis-worker.ts instead of
swallowing them, so they propagate through worker -> main thread ->
error event chain for proper visibility
- Replace allViolations.push(...spread) with for-of loop to avoid
potential stack overflow with very large violation arrays
- Strengthen test: verify exact error event count in "emits error" test
- Add tests: eachUrl error resilience, multi-plugin partial failure,
getHtml() rejection handling
https://claude.ai/code/session_01GGS8WVVNwxG2ueyEdyVUYW
- Add unit tests for 4 untested analyze plugins: - analyze-main-contents (6 tests): main detection, headings, images, tables - analyze-markuplint (7 tests): violations, URL naming, error handling - analyze-search (9 tests): keywords, selectors, scope, headers - analyze-textlint (6 tests): violations, severity mapping, rule merging - Create CONTRIBUTING.md with dev setup, workflow, and coding conventions - Create .env.example documenting Google OAuth2 credential setup - Add external dependency reference section to ARCHITECTURE.md with @d-zero/* package versions, usage map, and update notes - Optimize CI: share build artifacts between build and test jobs via actions/upload-artifact to eliminate redundant rebuild Test coverage: 79 → 83 files, 462 → 490 tests https://claude.ai/code/session_01GGS8WVVNwxG2ueyEdyVUYW
- Fix duplicate section 9 numbering (External Dependencies → section 13) - Complete dependency graph (add analyze-* → core and @d-zero/dealer edges) - Fix filesystem file count (14 → 16) - Remove specific version numbers from external deps table per project rules - Add missing @d-zero/fs and @d-zero/readtext to usage map https://claude.ai/code/session_01GGS8WVVNwxG2ueyEdyVUYW
YusukeHirao
force-pushed
the
claude/fix-analyze-empty-results-7ieOv
branch
from
b8ac0b8 to
87b4c41
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds robust error handling to the
analyze()method and comprehensive test coverage for the analyze pipeline, including all analyze plugins. Error handling now gracefully continues processing when individual pages or plugins fail, emits error events for visibility, and warns when plugins produce no data.closes: #35
Key Changes
Core Error Handling (
nitpicker.ts)Worker Error Propagation (
worker.ts,page-analysis-worker.ts)page-analysis-worker.tsthrows errors with plugin name prefix instead of logging to consolerunInWorker()to be handled by the main analyze loopTest Coverage
analyze()method covering:analyze-textlint: 216 lines covering linting, severity conversion, custom rulesanalyze-search: 201 lines covering keyword/selector search, scope restrictionanalyze-main-contents: 178 lines covering content detection and countinganalyze-markuplint: 174 lines covering HTML validation and URL normalizationDocumentation
CONTRIBUTING.mdwith development setup, workflow, coding conventions, and external dependenciesARCHITECTURE.mdwith external dependency reference table and worker thread error handling detailsCI/CD
Implementation Details
https://claude.ai/code/session_01GGS8WVVNwxG2ueyEdyVUYW