Skip to content

Commit ad03f1e

Browse files
chrisgavinchrisgavin
authored
Merge pull request #72 from github/better-http-errors
Include request IDs in errors when available.
2 parents d451bb4 + 3fd082e commit ad03f1e

File tree

4 files changed

+57
-28
lines changed

4 files changed

+57
-28
lines changed

‎internal/githubapiutil/githubapiutil.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ import (
44
"strings"
55

66
"github.com/google/go-github/v32/github"
7+
"github.com/pkg/errors"
78
)
89

910
const xOAuthScopesHeader = "X-OAuth-Scopes"
11+
const xGitHubRequestIDHeader = "X-GitHub-Request-ID"
1012

1113
func HasAnyScope(response *github.Response, scopes ...string) bool {
1214
if response == nil {
@@ -26,3 +28,14 @@ func HasAnyScope(response *github.Response, scopes ...string) bool {
2628
}
2729
return false
2830
}
31+
32+
func EnrichResponseError(response *github.Response, err error, message string) error {
33+
requestID := ""
34+
if response != nil {
35+
requestID = response.Header.Get(xGitHubRequestIDHeader)
36+
}
37+
if requestID != "" {
38+
message = message + " (" + requestID + ")"
39+
}
40+
return errors.Wrap(err, message)
41+
}

‎internal/githubapiutil/githubapiutil_test.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package githubapiutil
22

33
import (
4+
"errors"
45
"net/http"
56
"testing"
67

@@ -23,3 +24,16 @@ func TestHasAnyScope(t *testing.T) {
2324
response.Header.Set(xOAuthScopesHeader, "gist, notifications, admin:org")
2425
require.False(t, HasAnyScope(&response, "public_repo", "repo"))
2526
}
27+
28+
func TestEnrichErrorResponse(t *testing.T) {
29+
response := github.Response{
30+
Response: &http.Response{Header: http.Header{}},
31+
}
32+
33+
require.Equal(t, "The error message.: The underlying error.", EnrichResponseError(nil, errors.New("The underlying error."), "The error message.").Error())
34+
35+
require.Equal(t, "The error message.: The underlying error.", EnrichResponseError(&response, errors.New("The underlying error."), "The error message.").Error())
36+
37+
response.Header.Set(xGitHubRequestIDHeader, "AAAA:BBBB:CCCCCCC:DDDDDDD:EEEEEEEE")
38+
require.Equal(t, "The error message. (AAAA:BBBB:CCCCCCC:DDDDDDD:EEEEEEEE): The underlying error.", EnrichResponseError(&response, errors.New("The underlying error."), "The error message.").Error())
39+
}

‎internal/pull/pull.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
log "github.com/sirupsen/logrus"
1414

1515
"github.com/github/codeql-action-sync/internal/actionconfiguration"
16+
"github.com/github/codeql-action-sync/internal/githubapiutil"
1617
"github.com/mitchellh/ioprogress"
1718
"golang.org/x/oauth2"
1819

@@ -191,9 +192,9 @@ func (pullService *pullService) pullReleases() error {
191192

192193
for index, releaseTag := range relevantReleases {
193194
log.Debugf("Pulling CodeQL bundle %s (%d/%d)...", releaseTag, index+1, len(relevantReleases))
194-
release, _, err := pullService.githubDotComClient.Repositories.GetReleaseByTag(pullService.ctx, sourceOwner, sourceRepository, releaseTag)
195+
release, response, err := pullService.githubDotComClient.Repositories.GetReleaseByTag(pullService.ctx, sourceOwner, sourceRepository, releaseTag)
195196
if err != nil {
196-
return errors.Wrap(err, "Error loading CodeQL release information.")
197+
return githubapiutil.EnrichResponseError(response, err, "Error loading CodeQL release information.")
197198
}
198199
err = os.MkdirAll(pullService.cacheDirectory.ReleasePath(releaseTag), 0755)
199200
if err != nil {

‎internal/push/push.go‎

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func (pushService *pushService) createRepository() (*github.Repository, error) {
7272
if response != nil && response.StatusCode == http.StatusUnauthorized {
7373
return nil, usererrors.New(errorInvalidDestinationToken)
7474
}
75-
return nil, errors.Wrap(err, "Error getting current user.")
75+
return nil, githubapiutil.EnrichResponseError(response, err, "Error getting current user.")
7676
}
7777

7878
// When creating a repository we can either create it in a named organization or under the current user (represented in go-github by an empty string).
@@ -84,39 +84,39 @@ func (pushService *pushService) createRepository() (*github.Repository, error) {
8484
if destinationOrganization != "" {
8585
_, response, err := pushService.githubEnterpriseClient.Organizations.Get(pushService.ctx, pushService.destinationRepositoryOwner)
8686
if err != nil && (response == nil || response.StatusCode != http.StatusNotFound) {
87-
return nil, errors.Wrap(err, "Error checking if destination organization exists.")
87+
return nil, githubapiutil.EnrichResponseError(response, err, "Error checking if destination organization exists.")
8888
}
8989
if response != nil && response.StatusCode == http.StatusNotFound {
9090
log.Debugf("The organization %s does not exist. Creating it...", pushService.destinationRepositoryOwner)
91-
_, _, err := pushService.githubEnterpriseClient.Admin.CreateOrg(pushService.ctx, &github.Organization{
91+
_, response, err := pushService.githubEnterpriseClient.Admin.CreateOrg(pushService.ctx, &github.Organization{
9292
Login: github.String(pushService.destinationRepositoryOwner),
9393
Name: github.String(pushService.destinationRepositoryOwner),
9494
}, user.GetLogin())
9595
if err != nil {
9696
if response != nil && response.StatusCode == http.StatusNotFound && !githubapiutil.HasAnyScope(response, "site_admin") {
9797
return nil, usererrors.New("The destination token you have provided does not have the `site_admin` scope, so the destination organization cannot be created.")
9898
}
99-
return nil, errors.Wrap(err, "Error creating organization.")
99+
return nil, githubapiutil.EnrichResponseError(response, err, "Error creating organization.")
100100
}
101101
}
102102

103103
_, response, err = pushService.githubEnterpriseClient.Organizations.IsMember(pushService.ctx, pushService.destinationRepositoryOwner, user.GetLogin())
104104
if err != nil {
105-
return nil, errors.Wrap(err, "Failed to check membership of destination organization.")
105+
return nil, githubapiutil.EnrichResponseError(response, err, "Failed to check membership of destination organization.")
106106
}
107107
if (response.StatusCode == http.StatusFound || response.StatusCode == http.StatusNotFound) && githubapiutil.HasAnyScope(response, "site_admin") {
108108
log.Debugf("No access to destination organization (status code %d). Switching to impersonation token for %s...", response.StatusCode, pushService.actionsAdminUser)
109-
impersonationToken, _, err := pushService.githubEnterpriseClient.Admin.CreateUserImpersonation(pushService.ctx, pushService.actionsAdminUser, &github.ImpersonateUserOptions{Scopes: []string{minimumRepositoryScope, "workflow"}})
109+
impersonationToken, response, err := pushService.githubEnterpriseClient.Admin.CreateUserImpersonation(pushService.ctx, pushService.actionsAdminUser, &github.ImpersonateUserOptions{Scopes: []string{minimumRepositoryScope, "workflow"}})
110110
if err != nil {
111-
return nil, errors.Wrap(err, "Failed to impersonate Actions admin user.")
111+
return nil, githubapiutil.EnrichResponseError(response, err, "Failed to impersonate Actions admin user.")
112112
}
113113
pushService.destinationToken.AccessToken = impersonationToken.GetToken()
114114
}
115115
}
116116

117117
repository, response, err := pushService.githubEnterpriseClient.Repositories.Get(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName)
118118
if err != nil && (response == nil || response.StatusCode != http.StatusNotFound) {
119-
return nil, errors.Wrap(err, "Error checking if destination repository exists.")
119+
return nil, githubapiutil.EnrichResponseError(response, err, "Error checking if destination repository exists.")
120120
}
121121
if response.StatusCode != http.StatusNotFound && repositoryHomepage != repository.GetHomepage() && !pushService.force {
122122
return nil, errors.Errorf(errorAlreadyExists)
@@ -143,7 +143,7 @@ func (pushService *pushService) createRepository() (*github.Repository, error) {
143143
if response.StatusCode == http.StatusNotFound && !githubapiutil.HasAnyScope(response, acceptableRepositoryScopes...) {
144144
return nil, fmt.Errorf("The destination token you have provided does not have the `%s` scope.", minimumRepositoryScope)
145145
}
146-
return nil, errors.Wrap(err, "Error creating destination repository.")
146+
return nil, githubapiutil.EnrichResponseError(response, err, "Error creating destination repository.")
147147
}
148148
} else {
149149
log.Debug("Repository already exists. Updating its metadata...")
@@ -156,7 +156,7 @@ func (pushService *pushService) createRepository() (*github.Repository, error) {
156156
return nil, fmt.Errorf("You don't have permission to update the repository at %s/%s. If you wish to update the bundled CodeQL Action please provide a token with the `site_admin` scope.", pushService.destinationRepositoryOwner, pushService.destinationRepositoryName)
157157
}
158158
}
159-
return nil, errors.Wrap(err, "Error updating destination repository.")
159+
return nil, githubapiutil.EnrichResponseError(response, err, "Error updating destination repository.")
160160
}
161161
}
162162

@@ -212,6 +212,7 @@ func (pushService *pushService) pushGit(repository *github.Repository, initialPu
212212
}
213213
refSpecBatches = append(refSpecBatches, deleteRefSpecs)
214214

215+
defaultBranchRefSpec := "+refs/heads/main:refs/heads/main"
215216
if initialPush {
216217
releasePathStats, err := ioutil.ReadDir(pushService.cacheDirectory.ReleasesPath())
217218
if err != nil {
@@ -228,10 +229,10 @@ func (pushService *pushService) pushGit(repository *github.Repository, initialPu
228229
}
229230
refSpecBatches = append(refSpecBatches, initialRefSpecs)
230231
} else {
231-
// We've got to push `main` on its own, so that it will be made the default branch if the repository has just been created. We then push everything else afterwards.
232+
// We've got to push the default branch on its own, so that it will be made the default branch if the repository has just been created. We then push everything else afterwards.
232233
refSpecBatches = append(refSpecBatches,
233234
[]config.RefSpec{
234-
config.RefSpec("+refs/heads/main:refs/heads/main"),
235+
config.RefSpec(defaultBranchRefSpec),
235236
},
236237
[]config.RefSpec{
237238
config.RefSpec("+refs/*:refs/*"),
@@ -270,20 +271,20 @@ func (pushService *pushService) createOrUpdateRelease(releaseName string) (*gith
270271

271272
release, response, err := pushService.githubEnterpriseClient.Repositories.GetReleaseByTag(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, releaseMetadata.GetTagName())
272273
if err != nil && response.StatusCode != http.StatusNotFound {
273-
return nil, errors.Wrap(err, "Error checking for existing CodeQL release.")
274+
return nil, githubapiutil.EnrichResponseError(response, err, "Error checking for existing CodeQL release.")
274275
}
275276
if release == nil {
276277
log.Debugf("Creating release %s...", releaseMetadata.GetTagName())
277-
release, _, err := pushService.githubEnterpriseClient.Repositories.CreateRelease(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, &releaseMetadata)
278+
release, response, err := pushService.githubEnterpriseClient.Repositories.CreateRelease(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, &releaseMetadata)
278279
if err != nil {
279-
return nil, errors.Wrap(err, "Error creating release.")
280+
return nil, githubapiutil.EnrichResponseError(response, err, "Error creating release.")
280281
}
281282
return release, nil
282283
}
283-
release, _, err = pushService.githubEnterpriseClient.Repositories.EditRelease(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, release.GetID(), &releaseMetadata)
284+
release, response, err = pushService.githubEnterpriseClient.Repositories.EditRelease(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, release.GetID(), &releaseMetadata)
284285
if err != nil {
285286
log.Debugf("Updating release %s...", releaseMetadata.GetTagName())
286-
return nil, errors.Wrap(err, "Error updating release.")
287+
return nil, githubapiutil.EnrichResponseError(response, err, "Error updating release.")
287288
}
288289
return release, nil
289290
}
@@ -301,7 +302,7 @@ func (pushService *pushService) uploadReleaseAsset(release *github.RepositoryRel
301302
asset := &github.ReleaseAsset{}
302303
response, err := pushService.githubEnterpriseClient.Do(pushService.ctx, request, asset)
303304
if err != nil {
304-
return nil, response, errors.Wrap(err, "Error uploading release asset.")
305+
return nil, response, githubapiutil.EnrichResponseError(response, err, "Error uploading release asset.")
305306
}
306307
return asset, response, nil
307308
}
@@ -315,9 +316,9 @@ func (pushService *pushService) createOrUpdateReleaseAsset(release *github.Repos
315316
return nil
316317
} else {
317318
log.Warnf("Removing existing release asset %s because it was only partially-uploaded (had size %d, but should have been %d)...", existingAsset.GetName(), actualSize, expectedSize)
318-
_, err := pushService.githubEnterpriseClient.Repositories.DeleteReleaseAsset(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, existingAsset.GetID())
319+
response, err := pushService.githubEnterpriseClient.Repositories.DeleteReleaseAsset(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, existingAsset.GetID())
319320
if err != nil {
320-
return errors.Wrap(err, "Error deleting existing release asset.")
321+
return githubapiutil.EnrichResponseError(response, err, "Error deleting existing release asset.")
321322
}
322323
}
323324
}
@@ -333,9 +334,9 @@ func (pushService *pushService) createOrUpdateReleaseAsset(release *github.Repos
333334
Size: assetPathStat.Size(),
334335
DrawFunc: ioprogress.DrawTerminalf(os.Stderr, ioprogress.DrawTextFormatBytes),
335336
}
336-
_, _, err = pushService.uploadReleaseAsset(release, assetPathStat, progressReader)
337+
_, response, err := pushService.uploadReleaseAsset(release, assetPathStat, progressReader)
337338
if err != nil {
338-
return errors.Wrap(err, "Error uploading release asset.")
339+
return githubapiutil.EnrichResponseError(response, err, "Error uploading release asset.")
339340
}
340341
return nil
341342
}
@@ -358,9 +359,9 @@ func (pushService *pushService) pushReleases() error {
358359

359360
existingAssets := []*github.ReleaseAsset{}
360361
for page := 1; ; page++ {
361-
assets, _, err := pushService.githubEnterpriseClient.Repositories.ListReleaseAssets(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, release.GetID(), &github.ListOptions{Page: page})
362+
assets, response, err := pushService.githubEnterpriseClient.Repositories.ListReleaseAssets(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, release.GetID(), &github.ListOptions{Page: page})
362363
if err != nil {
363-
return errors.Wrap(err, "Error fetching existing release assets.")
364+
return githubapiutil.EnrichResponseError(response, err, "Error fetching existing release assets.")
364365
}
365366
if len(assets) == 0 {
366367
break
@@ -376,7 +377,7 @@ func (pushService *pushService) pushReleases() error {
376377
for _, assetPathStat := range assetPathStats {
377378
err := pushService.createOrUpdateReleaseAsset(release, existingAssets, assetPathStat)
378379
if err != nil {
379-
return errors.Wrap(err, "Error uploading release assets.")
380+
return err
380381
}
381382
}
382383
}
@@ -410,7 +411,7 @@ func Push(ctx context.Context, cacheDirectory cachedirectory.CacheDirectory, des
410411
}
411412
rootResponse, err := client.Do(ctx, rootRequest, nil)
412413
if err != nil {
413-
return errors.Wrap(err, "Error checking connectivity for GitHub Enterprise client.")
414+
return githubapiutil.EnrichResponseError(rootResponse, err, "Error checking connectivity for GitHub Enterprise client.")
414415
}
415416
if rootRequest.URL != rootResponse.Request.URL {
416417
updatedBaseURL, _ := url.Parse(client.BaseURL.String())

0 commit comments

Comments
 (0)