Skip to content

fix(general): Improve error handling to surface the most relevant error#6728

Open
PeterSchafer wants to merge 2 commits intomainfrom
fix/CLI-1455_error_priority
Open

fix(general): Improve error handling to surface the most relevant error#6728
PeterSchafer wants to merge 2 commits intomainfrom
fix/CLI-1455_error_priority

Conversation

@PeterSchafer
Copy link
Copy Markdown
Contributor

Pull Request Submission Checklist

  • Follows CONTRIBUTING guidelines
  • Commit messages are release-note ready, emphasizing what was changed, not how.
  • Includes detailed description of changes
  • Contains risk assessment (Low | Medium | High)
  • Highlights breaking API changes (if applicable)
  • Links to automated tests covering new functionality
  • Includes manual testing instructions (if necessary)
  • Updates relevant GitBook documentation (PR link: ___)
  • Includes product update to be announced in the next stable release notes

What does this PR do?

Implements error prioritization to ensure the most relevant/severe error is displayed and used for exit code derivation when multiple errors occur during CLI execution.

Changes:

  • Added FindMostRelevantError() function that finds the most relevant error from a list based on:
    1. snyk_errors.Error over plain errors
    2. Error Level (error > warn > info > unset)
    3. StatusCode (higher status codes first)
  • Updated error processing to use FindMostRelevantError() for selecting which error to display and derive exit codes from
  • Properly handles joined errors (from errors.Join()) by recursively inspecting nested errors while preserving the original error structure/relationships
  • Moved maintenance error tests to a dedicated test file

Where should the reviewer start?

  1. Core logic for FindMostRelevantError() and priority comparison in the CLI errors package
  2. Usage in the processError() function
  3. Unit tests covering various scenarios

How should this be manually tested?

npx jest test/jest/acceptance/maintenance.spec.ts

@PeterSchafer PeterSchafer requested review from a team as code owners April 15, 2026 15:22
@snyk-io
Copy link
Copy Markdown

snyk-io bot commented Apr 15, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@snyk-pr-review-bot

This comment has been minimized.

@PeterSchafer PeterSchafer force-pushed the fix/CLI-1455_error_priority branch 3 times, most recently from bfa4b5d to 6d3b8d2 Compare April 15, 2026 16:40
@snyk-pr-review-bot

This comment has been minimized.

@PeterSchafer PeterSchafer force-pushed the fix/CLI-1455_error_priority branch from 6d3b8d2 to 0e01700 Compare April 15, 2026 16:52
@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot

This comment has been minimized.

@github-actions
Copy link
Copy Markdown
Contributor

Warnings
⚠️ There are multiple commits on your branch, please squash them locally before merging!

Generated by 🚫 dangerJS against ea58212

@snyk-pr-review-bot
Copy link
Copy Markdown

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Hardcoded Error Code 🟡 [minor]

The findSnykErrorByCode function and maintenanceErrorCode variable use a hardcoded string "SNYK-0099" derived from snyk.NewMaintenanceWindowError("").ErrorCode. While this currently works, it creates a tight coupling. If the error catalog were to change the format or value of this specific maintenance error code in the future, this logic would silently fail to identify it, leading to incorrect error prioritization and exit codes. It would be more robust to reference a constant or an enumeration for maintenanceErrorCode if one exists in the snyk_errors package, or provide a mechanism for dynamic updates.

var maintenanceErrorCode = snyk.NewMaintenanceWindowError("").ErrorCode
📚 Repository Context Analyzed

This review considered 14 relevant code sections from 10 files (average relevance: 0.86)

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.

1 participant