Skip to content

Commit 6ab28ef

Browse files
authored
Merge pull request #36 from github/prettier-errors
Make common errors easier to read.
2 parents 45499fa + 3a6d26b commit 6ab28ef

File tree

5 files changed

+83
-13
lines changed

5 files changed

+83
-13
lines changed

internal/cachedirectory/cachedirectory.go

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package cachedirectory
22

33
import (
4-
"fmt"
4+
usererrors "errors"
55
"io"
66
"io/ioutil"
77
"os"
@@ -15,7 +15,8 @@ import (
1515
const errorCacheWrongVersion = "The cache you are trying to push was created with an old version of the CodeQL Action Sync tool. Please re-pull it with this version of the tool."
1616
const errorNotACacheOrEmpty = "The cache directory you have selected is not empty, but was not created by the CodeQL Action Sync tool. If you are sure you want to use this directory, please delete it and run the sync tool again."
1717
const errorCacheParentDoesNotExist = "Cannot create cache directory because its parent, does not exist."
18-
const errorPushNonCache = "The directory you have provided does not appear to be valid. Please check it exists and that you have run the `pull` command to populate it."
18+
const errorPushNonCache = "The cache directory you have provided does not appear to be valid. Please check it exists and that you have run the `pull` command to populate it."
19+
const errorCacheLocked = "The cache directory is locked, likely due to a `pull` command being interrupted. Please run `pull` again to ensure all required data is downloaded."
1920

2021
const CacheReferencePrefix = "refs/remotes/" + git.DefaultRemoteName + "/"
2122

@@ -35,7 +36,7 @@ func isEmptyOrNonExistentDirectory(path string) (bool, error) {
3536
if os.IsNotExist(err) {
3637
return true, nil
3738
}
38-
return false, errors.Wrap(err, fmt.Sprintf("Could not access directory %s.", path))
39+
return false, errors.Wrapf(err, "Could not access directory %s.", path)
3940
}
4041
defer f.Close()
4142

@@ -44,7 +45,7 @@ func isEmptyOrNonExistentDirectory(path string) (bool, error) {
4445
if err == io.EOF {
4546
return true, nil
4647
}
47-
return false, errors.Wrap(err, fmt.Sprintf("Could not read contents of directory %s.", path))
48+
return false, errors.Wrapf(err, "Could not read contents of directory %s.", path)
4849
}
4950
return false, nil
5051
}
@@ -67,7 +68,7 @@ func (cacheDirectory *CacheDirectory) CheckOrCreateVersionFile(pull bool, versio
6768
_, err := os.Stat(cacheParentPath)
6869
if err != nil {
6970
if os.IsNotExist(err) {
70-
return errors.New(errorCacheParentDoesNotExist)
71+
return usererrors.New(errorCacheParentDoesNotExist)
7172
}
7273
return errors.Wrap(err, "Could not access parent path of cache directory.")
7374
}
@@ -94,13 +95,13 @@ func (cacheDirectory *CacheDirectory) CheckOrCreateVersionFile(pull bool, versio
9495
}
9596
return nil
9697
}
97-
return errors.New(errorNotACacheOrEmpty)
98+
return usererrors.New(errorNotACacheOrEmpty)
9899
}
99100

100101
if cacheVersionFileExists {
101-
return errors.New(errorCacheWrongVersion)
102+
return usererrors.New(errorCacheWrongVersion)
102103
}
103-
return errors.New(errorPushNonCache)
104+
return usererrors.New(errorPushNonCache)
104105
}
105106

106107
func (cacheDirectory *CacheDirectory) Lock() error {
@@ -124,7 +125,7 @@ func (cacheDirectory *CacheDirectory) Unlock() error {
124125
func (cacheDirectory *CacheDirectory) CheckLock() error {
125126
_, err := os.Stat(cacheDirectory.lockFilePath())
126127
if err == nil {
127-
return errors.New("The cache directory is locked, likely due to a `pull` command being interrupted. Please run `pull` again to ensure all required data is downloaded.")
128+
return usererrors.New(errorCacheLocked)
128129
}
129130
if os.IsNotExist(err) {
130131
return nil

internal/cachedirectory/cachedirectory_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ func TestLocking(t *testing.T) {
9595
require.NoError(t, cacheDirectory.CheckOrCreateVersionFile(true, aVersion))
9696
require.NoError(t, cacheDirectory.Lock())
9797
require.NoError(t, cacheDirectory.Lock())
98-
require.Error(t, cacheDirectory.CheckLock())
98+
require.EqualError(t, cacheDirectory.CheckLock(), errorCacheLocked)
9999
require.NoError(t, cacheDirectory.Unlock())
100100
require.NoError(t, cacheDirectory.CheckLock())
101101
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package githubapiutil
2+
3+
import (
4+
"strings"
5+
6+
"github.com/google/go-github/v32/github"
7+
)
8+
9+
const xOAuthScopesHeader = "X-OAuth-Scopes"
10+
11+
func MissingAllScopes(response *github.Response, requiredAnyScopes ...string) bool {
12+
if response == nil {
13+
return false
14+
}
15+
if len(response.Header.Values(xOAuthScopesHeader)) == 0 {
16+
return false
17+
}
18+
actualScopes := strings.Split(response.Header.Get(xOAuthScopesHeader), ",")
19+
for _, actualScope := range actualScopes {
20+
actualScope = strings.Trim(actualScope, " ")
21+
for _, requiredAnyScope := range requiredAnyScopes {
22+
if actualScope == requiredAnyScope {
23+
return false
24+
}
25+
}
26+
}
27+
return true
28+
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package githubapiutil
2+
3+
import (
4+
"net/http"
5+
"testing"
6+
7+
"github.com/stretchr/testify/require"
8+
9+
"github.com/google/go-github/v32/github"
10+
)
11+
12+
func TestHasAnyScopes(t *testing.T) {
13+
response := github.Response{
14+
Response: &http.Response{Header: http.Header{}},
15+
}
16+
17+
response.Header.Set(xOAuthScopesHeader, "gist, notifications, admin:org, repo")
18+
require.False(t, MissingAllScopes(&response, "public_repo", "repo"))
19+
20+
response.Header.Set(xOAuthScopesHeader, "gist, notifications, public_repo, admin:org")
21+
require.False(t, MissingAllScopes(&response, "public_repo", "repo"))
22+
23+
response.Header.Set(xOAuthScopesHeader, "gist, notifications, admin:org")
24+
require.True(t, MissingAllScopes(&response, "public_repo", "repo"))
25+
}

internal/push/push.go

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package push
33
import (
44
"context"
55
"encoding/json"
6+
usererrors "errors"
67
"fmt"
78
"io"
89
"io/ioutil"
@@ -13,6 +14,8 @@ import (
1314
"path/filepath"
1415
"strings"
1516

17+
"github.com/github/codeql-action-sync/internal/githubapiutil"
18+
1619
log "github.com/sirupsen/logrus"
1720

1821
"github.com/github/codeql-action-sync/internal/cachedirectory"
@@ -30,6 +33,7 @@ const remoteName = "enterprise"
3033
const repositoryHomepage = "https://github.com/github/codeql-action-sync-tool/"
3134

3235
const errorAlreadyExists = "The destination repository already exists, but it was not created with the CodeQL Action sync tool. If you are sure you want to push the CodeQL Action to it, re-run this command with the `--force` flag."
36+
const errorInvalidDestinationToken = "The destination token you've provided is not valid."
3337

3438
type pushService struct {
3539
ctx context.Context
@@ -43,8 +47,11 @@ type pushService struct {
4347

4448
func (pushService *pushService) createRepository() (*github.Repository, error) {
4549
log.Debug("Ensuring repository exists...")
46-
user, _, err := pushService.githubEnterpriseClient.Users.Get(pushService.ctx, "")
50+
user, response, err := pushService.githubEnterpriseClient.Users.Get(pushService.ctx, "")
4751
if err != nil {
52+
if response.StatusCode == http.StatusUnauthorized {
53+
return nil, usererrors.New(errorInvalidDestinationToken)
54+
}
4855
return nil, errors.Wrap(err, "Error getting current user.")
4956
}
5057

@@ -66,6 +73,9 @@ func (pushService *pushService) createRepository() (*github.Repository, error) {
6673
Name: github.String(pushService.destinationRepositoryOwner),
6774
}, user.GetLogin())
6875
if err != nil {
76+
if response.StatusCode == http.StatusNotFound && githubapiutil.MissingAllScopes(response, "site_admin") {
77+
return nil, usererrors.New("The destination token you have provided does not have the `site_admin` scope, so the destination organization cannot be created.")
78+
}
6979
return nil, errors.Wrap(err, "Error creating organization.")
7080
}
7181
}
@@ -90,13 +100,19 @@ func (pushService *pushService) createRepository() (*github.Repository, error) {
90100
Private: github.Bool(false),
91101
}
92102
if response.StatusCode == http.StatusNotFound {
93-
repository, _, err = pushService.githubEnterpriseClient.Repositories.Create(pushService.ctx, destinationOrganization, &desiredRepositoryProperties)
103+
repository, response, err = pushService.githubEnterpriseClient.Repositories.Create(pushService.ctx, destinationOrganization, &desiredRepositoryProperties)
94104
if err != nil {
105+
if response.StatusCode == http.StatusNotFound && githubapiutil.MissingAllScopes(response, "public_repo", "repo") {
106+
return nil, usererrors.New("The destination token you have provided does not have the `public_repo` scope.")
107+
}
95108
return nil, errors.Wrap(err, "Error creating destination repository.")
96109
}
97110
} else {
98-
repository, _, err = pushService.githubEnterpriseClient.Repositories.Edit(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, &desiredRepositoryProperties)
111+
repository, response, err = pushService.githubEnterpriseClient.Repositories.Edit(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, &desiredRepositoryProperties)
99112
if err != nil {
113+
if response.StatusCode == http.StatusNotFound && githubapiutil.MissingAllScopes(response, "public_repo", "repo") {
114+
return nil, usererrors.New("The destination token you have provided does not have the `public_repo` scope.")
115+
}
100116
return nil, errors.Wrap(err, "Error updating destination repository.")
101117
}
102118
}

0 commit comments

Comments
 (0)