Skip to content

refactor(errors): extract network hint dispatch and formatting#824

Merged
sonukapoor merged 5 commits into
OWASP:mainfrom
luojiyin1987:refactor/cli-errors-formatter
Jul 9, 2026
Merged

refactor(errors): extract network hint dispatch and formatting#824
sonukapoor merged 5 commits into
OWASP:mainfrom
luojiyin1987:refactor/cli-errors-formatter

Conversation

@luojiyin1987

@luojiyin1987 luojiyin1987 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

What

Extract network error hint dispatch and hint-line formatting out of src/index.ts.

getNetworkErrorHint() now lives in src/utils/network.ts, and formatHintLines() now lives in src/output/formatters.ts.

Why

  • Eliminates 5 identical if/else branches that each repeat the same chalk formatting
  • Provides reusable error handling utilities for the upcoming dispatch layer (PR 5)
  • Keeps network hint dispatch in utils/network.ts
  • Keeps chalk-based formatting in output/formatters.ts

Changes

  • src/utils/network.ts: add getNetworkErrorHint() for network hint dispatch
  • src/output/formatters.ts: add formatHintLines() for shared hint formatting
  • src/index.ts: replace repeated network hint if/else block with composed helper calls
  • Tests: cover all getNetworkErrorHint() branches and formatHintLines() formatting behavior

Testing

  • TypeScript compiles cleanly
  • No behavior change (same hints, same formatting, same fallback to fetchErrorCaCertHint)

Closes #823

@luojiyin1987 luojiyin1987 requested a review from sonukapoor as a code owner July 8, 2026 02:50

@sonukapoor sonukapoor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for picking this up - the 20-line if/else block is genuine duplication and extracting it is the right call. The routing logic is clean and the behavioral output is preserved exactly.

My main concern is the module placement. formatHintLines is a chalk formatting utility - it does the same work as formatRelationshipLabel and formatAdvisorySourceLine in formatters.ts, so that's where it belongs. And getNetworkErrorHint has no chalk dependency at all - it only calls functions that already live in network.ts, so it fits there naturally. Those two moves achieve the full index.ts cleanup you're after without needing a new cli-errors.ts file.

The other gap is tests. Every detection function in network.ts is covered in tests/network.test.ts - the two new functions should get the same treatment. formatHintLines has an empty-input branch, getNetworkErrorHint has five branches, both need coverage.

@luojiyin1987

Copy link
Copy Markdown
Collaborator Author

Thanks, that makes sense. I’ll drop the new cli-errors module and move the helpers into the existing boundaries: formatHintLines into output/formatters.ts, and getNetworkErrorHint into utils/network.ts.

I’ll also add coverage for the empty formatHintLines input case and all five getNetworkErrorHint branches.

@luojiyin1987 luojiyin1987 changed the title refactor(cli-errors): extract error hint formatting and network error dispatch refactor(errors): extract network hint dispatch and formatting Jul 8, 2026

@sonukapoor sonukapoor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exactly what I was looking for - formatHintLines in formatters.ts alongside the other chalk formatting helpers, getNetworkErrorHint in network.ts where all the detection logic already lives, and no new file needed. The test coverage is thorough too: all five branches of getNetworkErrorHint in network.test.ts and both paths of formatHintLines in output.test.ts. Well done.

@sonukapoor sonukapoor merged commit 5200a33 into OWASP:main Jul 9, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(cli-errors): extract error hint formatting and network error dispatch

2 participants