-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes issue description and octokit pagination #28
Fixes issue description and octokit pagination #28
Conversation
…s previous queryOrganization funtion.
|
||
const baseV1Url = 'https://snyk.io/api/v1' | ||
const baseRestUrl = 'https://api.snyk.io' | ||
const baseRestUrl = 'https://api.snyk.io/rest' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided to include the full base here and just remove the '/rest' in the one place where it is redundant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jeramysoucy I think this is now introducing this when used Error: Failed to paginate request for get https://api.snyk.io/rest/rest/orgs/
, notice the double /rest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related issue: #30
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Changes LGTM!
++ descriptive errors!
Closes #27
Closes #29
Summary
This PR resolves the issue of undefined project links manifest version (see #27), as well as the issue where
octokit
pagination with API calls caused improper headers in low-level fetch in node >= 18 (see #29)Details
Undefined Values
The project links were derived from the response of the deprecated v1 all projects API - each project body contained a
browseUrl
property. According to the API migration documentation it:But this is inaccurate. The URL can be constructed from the organization name in lower-case and the project ID as such:
https://app.snyk.io/org/[org name]/project/[projectId]
The manifest version used to be populated from the
imageTag
property return from the deprecated v1 all projects API. The API migration documentation claims that it:But I found that this did not work. Example for several projects:
Originally, I removed the
manifest version
from the output issue description, as it is not included in our other issue-generation automations. However, I found that I could still properly query forimageTag
with the non-deprecated v1 single project API. As this API also returns thebrowseUrl
property, it is not necessary to query for the organization name and manually construct the URL.Fetch Header Issue
This PR fixes the fetch header issue by replacing the
octokit
API call within theoctokit
pagination call for querying issue labels with a raw request URL. This seems to resolve this issue and executes correctly in node 16, 18, and 20.Additional
Lastly, error reporting around requests has been improved throughout the codebase. Previously, only low-level exceptions were being displayed if something went wrong, which did not include a path or reference back to which high-level operation caused the error. Error reports will now include a better description, along with a response status code and response details if available.