feat: [TORCH-584] Reconstruct error-catalog errors from Test API errors - #769
bsalomon-snyk wants to merge 3 commits into
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
This comment has been minimized.
This comment has been minimized.
| // GetWarnings()) as error-catalog snyk_errors.Error values, reusing the same JSON:API | ||
| // parsing applied to non-2xx responses elsewhere in this client. Returns (nil, nil) | ||
| // when errs is nil or empty. | ||
| func ErrorsAsSnykErrors(errs *[]IoSnykApiCommonError) ([]snyk_errors.Error, error) { |
There was a problem hiding this comment.
Question: isn't this already working from the response middleware?
There was a problem hiding this comment.
No, unfortunately it only triggers on 2xx/3xx. Since Test API polls Test-API-Shim and gets back a 20x when the test ends, it doesn't process the upstream Snyk errors in errors[].
| return nil, nil | ||
| } | ||
|
|
||
| body, err := json.Marshal(map[string]any{"errors": *errs}) |
There was a problem hiding this comment.
From AI review:
"""
IoSnykApiCommonError.Links.About can be an object on the wire, but snyk_errors expects about as a string — re-marshaling the raw struct here will fail to parse (and drop all errors) if any error carries a links object. Consider stripping Links/Source before marshaling, or building a narrower local payload struct.
"""
There was a problem hiding this comment.
Thanks. Updated code to unmarshal only the fields TestAPI cares about so we don't encounter Links.About.
Note that the finding also applies to CLI's middleware, so error-catalog should get a fix for it some day. If CLI encounters an object in Links.About, it silently drops the error instead of returning it.
… codes from test result errors
- TestResult.GetErrors() can contain embedded errors while the Test Result fetch itself
gives a successful response body. The existing non-2xx error parsing in
handleUnexpectedResponse will not run for these, so callers need custom mapping.
- ErrorsAsSnykErrors reuses the parsing in snyk_errors.FromJSONAPIErrorBytes, so
callers get real snyk_errors.Error values with ErrorCode/Title/Detail intact.
a926b00 to
72b7dda
Compare
This comment has been minimized.
This comment has been minimized.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 72b7dda. Configure here.
72b7dda to
80e5f20
Compare
This comment has been minimized.
This comment has been minimized.
… box label
- populate meta.level so snyk_errors.FromJSONAPIErrorBytes can use it.
This translates into the CLI's error box label.
80e5f20 to
480aa54
Compare
PR Reviewer Guide 🔍
|
4 participants
Description
Add ErrorsAsSnykErrors to reconstruct error-catalog errors from Test API errors
TestResult.GetErrors() may contain embedded errors while the Test Result fetch itself gives a successful response body. The existing non-2xx error parsing in handleUnexpectedResponse will not run for these, so callers need custom mapping.
ErrorsAsSnykErrors gives CLI extensions a way to extract these into Snyk Catalog errors. It reuses the parsing in snyk_errors.FromJSONAPIErrorBytes, so callers get real snyk_errors.Error values with ErrorCode/Title/Detail intact.
Checklist
make test)make generate)make lint)go get github.com/snyk/go-application-framework@YOUR_LATEST_GAF_COMMITin thecliv2directory.go.modto point to your local GAF code.go mod tidyin thecliv2directory.go.modandgo.sumchanges.Note
Low Risk
Additive test API helper with focused tests; no changes to polling, auth, or existing error handling paths.
Overview
Adds
ErrorsAsSnykErrorsso callers can turnTestResult.GetErrors()payloads intosnyk_errors.Errorvalues using the sameFromJSONAPIErrorBytespath as non-2xx HTTP responses. That matters when a finished test returns 200 with errors in the result body—handleUnexpectedResponsenever runs in that case.Conversion builds a safe JSON:API
errorsdocument viajsonAPIErrorPayload, copying code/title/detail/status/id and settingmeta.leveltoerror, while omittingLinkssoLinks.Aboutunion shapes (string vs object) cannot break parsing. Empty or nil input returns(nil, nil).New integration-style tests cover full field mapping (e.g. quota
SNYK-0006), absent title, object-shapedlinks.about, and results with no errors.Reviewed by Cursor Bugbot for commit 480aa54. Bugbot is set up for automated code reviews on this repo. Configure here.