Skip to content

Conversation

@PeterSchafer
Copy link
Collaborator

@PeterSchafer PeterSchafer commented Jan 6, 2026

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?

This PR centralizes the mapping from errors to exit codes. This enables to consistently map errors no matter if they are part of the dataflow or returned from an extension.

Where should the reviewer start?

How should this be manually tested?

What's the product update that needs to be communicated to CLI users?

N/A

Risk assessment (Low | Medium | High)?

Medium, as it alters the flow of errors and exit codes. Looking into additional tests to reduce the risk more.

@PeterSchafer PeterSchafer requested review from a team as code owners January 6, 2026 15:01
@PeterSchafer PeterSchafer force-pushed the chore/CLI-1297_exitcode_mapping branch 5 times, most recently from 9d7664b to 1689497 Compare January 6, 2026 19:55
@PeterSchafer PeterSchafer force-pushed the chore/CLI-1297_exitcode_mapping branch from 1689497 to a7199b0 Compare January 7, 2026 09:37
@PeterSchafer PeterSchafer enabled auto-merge January 7, 2026 09:38
}

return defaultValue
}
Copy link
Member

@thisislawatts thisislawatts Jan 7, 2026

Choose a reason for hiding this comment

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

suggestion: How about using a static map, this would be something we could easily migrate out to configuration at a later date:

// errorCatalogToExitCodeMap centralizes the relationship between catalog errors and CLI exit codes.
var errorCatalogToExitCodeMap = map[string]int{
	code.NewUnsupportedProjectError("").ErrorCode: constants.SNYK_EXIT_CODE_UNSUPPORTED_PROJECTS,
    // Add new mappings here
}

var MapErrorCatalogToExitCode func(err *snyk_errors.Error, defaultValue int) int = mapErrorToExitCode

func mapErrorToExitCode(err *snyk_errors.Error, defaultValue int) int {
	if exitCode, internalExists := errorCatalogToExitCodeMap[err.ErrorCode]; internalExists {
		return exitCode
	}

	return defaultValue
}

Copy link
Member

@thisislawatts thisislawatts Jan 7, 2026

Choose a reason for hiding this comment

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

I discovered the Comma Ok pattern

Sometimes you need to distinguish a missing entry from a zero value. Is there an entry for "UTC" or is that 0 because it's not in the map at all? You can discriminate with a form of multiple assignment.

var seconds int
var ok bool
seconds, ok = timeZone[tz]

For obvious reasons this is called the “comma ok” idiom. In this example, if tz is present, seconds will be set appropriately and ok will be true; if not, seconds will be set to zero and ok will be false. Here's a function that puts it together with a nice error report:

func offset(tz string) int {
    if seconds, ok := timeZone[tz]; ok {
        return seconds
    }
    log.Println("unknown time zone:", tz)
    return 0
}

Source: https://go.dev/doc/effective_go

@PeterSchafer PeterSchafer force-pushed the chore/CLI-1297_exitcode_mapping branch from a7199b0 to 8c905c7 Compare January 7, 2026 10:44
@PeterSchafer PeterSchafer disabled auto-merge January 7, 2026 11:02
@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2026

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

Generated by 🚫 dangerJS against d8fed82

@j-luong j-luong force-pushed the chore/CLI-1297_exitcode_mapping branch 2 times, most recently from 1045017 to 6b450b9 Compare January 7, 2026 14:30
@j-luong j-luong enabled auto-merge January 7, 2026 14:34
@j-luong j-luong force-pushed the chore/CLI-1297_exitcode_mapping branch 2 times, most recently from 7b471ec to 95d7175 Compare January 7, 2026 21:02
@thisislawatts thisislawatts force-pushed the chore/CLI-1297_exitcode_mapping branch 2 times, most recently from a60a0ad to eb7f0c4 Compare January 8, 2026 09:00
@j-luong j-luong force-pushed the chore/CLI-1297_exitcode_mapping branch 2 times, most recently from d36963b to 3ad8cfb Compare January 8, 2026 11:16
@j-luong j-luong force-pushed the chore/CLI-1297_exitcode_mapping branch from 3ad8cfb to d8fed82 Compare January 8, 2026 11:27
@j-luong j-luong merged commit 22772d4 into main Jan 8, 2026
6 checks passed
@j-luong j-luong deleted the chore/CLI-1297_exitcode_mapping branch January 8, 2026 12:02
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.

4 participants