Fix up tests

api/client.go

@@ -132,13 +132,13 @@ func (c Client) RESTWithNext(hostname string, method string, p string, body io.R
 	return next, nil
 }
 
+// HandleHTTPError parses a http.Response into a HTTPError.
 func HandleHTTPError(resp *http.Response) error {
-	err := ghAPI.HandleHTTPError(resp)
-	return handleResponse(err)
+	return handleResponse(ghAPI.HandleHTTPError(resp))
 }
 
-//TODO: Rename this awful name
-// Maybe wrapAPIError?
+// handleResponse takes a ghAPI.HTTPError or ghAPI.GQLError and converts it into an
+// HTTPError or GraphQLError respectively.
 func handleResponse(err error) error {
 	if err == nil {
 		return nil
@@ -149,8 +149,8 @@ func handleResponse(err error) error {
 		return HTTPError{
 			HTTPError: restErr,
 			scopesSuggestion: generateScopesSuggestion(restErr.StatusCode,
-				restErr.AcceptedOAuthScopes,
-				restErr.OAuthScopes,
+				restErr.Headers.Get("X-Accepted-Oauth-Scopes"),
+				restErr.Headers.Get("X-Oauth-Scopes"),
 				restErr.RequestURL.Hostname()),
 		}
 	}

go.mod

@@ -9,7 +9,7 @@ require (
 	github.com/charmbracelet/glamour v0.4.0
 	github.com/charmbracelet/lipgloss v0.5.0
 	github.com/cli/browser v1.1.0
-	github.com/cli/go-gh v0.0.4-0.20220517195232-b7131b0759cc
+	github.com/cli/go-gh v0.0.4-0.20220523141704-14561a42d6e2
 	github.com/cli/oauth v0.9.0
 	github.com/cli/safeexec v1.0.0
 	github.com/cli/shurcooL-graphql v0.0.1

go.sum

@@ -58,8 +58,8 @@ github.com/cli/browser v1.1.0 h1:xOZBfkfY9L9vMBgqb1YwRirGu6QFaQ5dP/vXt5ENSOY=
 github.com/cli/browser v1.1.0/go.mod h1:HKMQAt9t12kov91Mn7RfZxyJQQgWgyS/3SZswlZ5iTI=
 github.com/cli/crypto v0.0.0-20210929142629-6be313f59b03 h1:3f4uHLfWx4/WlnMPXGai03eoWAI+oGHJwr+5OXfxCr8=
 github.com/cli/crypto v0.0.0-20210929142629-6be313f59b03/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
-github.com/cli/go-gh v0.0.4-0.20220517195232-b7131b0759cc h1:G3cDtFIsV5jI7TYim0K9jPEulruXqjbS3bcv0Ba4eeA=
-github.com/cli/go-gh v0.0.4-0.20220517195232-b7131b0759cc/go.mod h1:R/GsVfMGy+dmMjW4XIHFRhw8qAr2bF7bx2RtD93QZZE=
+github.com/cli/go-gh v0.0.4-0.20220523141704-14561a42d6e2 h1:O2OMzlSH69b/rDg8cu/E2lp0YuNFRAV/ey3/d87m9ww=
+github.com/cli/go-gh v0.0.4-0.20220523141704-14561a42d6e2/go.mod h1:R/GsVfMGy+dmMjW4XIHFRhw8qAr2bF7bx2RtD93QZZE=
 github.com/cli/oauth v0.9.0 h1:nxBC0Df4tUzMkqffAB+uZvisOwT3/N9FpkfdTDtafxc=
 github.com/cli/oauth v0.9.0/go.mod h1:qd/FX8ZBD6n1sVNQO3aIdRxeu5LGw9WhKnYhIIoC2A4=
 github.com/cli/safeexec v1.0.0 h1:0VngyaIyqACHdcMNWfo6+KdUYnqEr2Sg+bSP1pdF+dI=

internal/authflow/flow.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
 	"os"
 	"regexp"
 	"strings"
@@ -30,6 +31,7 @@ var (
 )
 
 type iconfig interface {
+	Get(string, string) (string, error)
 	Set(string, string, string) error
 	Write() error
 	WriteHosts() error
@@ -39,7 +41,16 @@ func AuthFlowWithConfig(cfg iconfig, IO *iostreams.IOStreams, hostname, notice s
 	// TODO this probably shouldn't live in this package. It should probably be in a new package that
 	// depends on both iostreams and config.
 
-	token, userLogin, err := authFlow(hostname, IO, notice, additionalScopes, isInteractive)
+	// FIXME: this duplicates `factory.browserLauncher()`
+	browserLauncher := os.Getenv("GH_BROWSER")
+	if browserLauncher == "" {
+		browserLauncher, _ = cfg.Get("", "browser")
+	}
+	if browserLauncher == "" {
+		browserLauncher = os.Getenv("BROWSER")
+	}
+
+	token, userLogin, err := authFlow(hostname, IO, notice, additionalScopes, isInteractive, browserLauncher)
 	if err != nil {
 		return "", err
 	}
@@ -56,7 +67,7 @@ func AuthFlowWithConfig(cfg iconfig, IO *iostreams.IOStreams, hostname, notice s
 	return token, cfg.WriteHosts()
 }
 
-func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, additionalScopes []string, isInteractive bool) (string, string, error) {
+func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, additionalScopes []string, isInteractive bool, browserLauncher string) (string, string, error) {
 	w := IO.ErrOut
 	cs := IO.ColorScheme()
 
@@ -87,25 +98,31 @@ func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, addition
 			fmt.Fprintf(w, "%s First copy your one-time code: %s\n", cs.Yellow("!"), cs.Bold(code))
 			return nil
 		},
-		BrowseURL: func(url string) error {
+		BrowseURL: func(authURL string) error {
+			if u, err := url.Parse(authURL); err == nil {
+				if u.Scheme != "http" && u.Scheme != "https" {
+					return fmt.Errorf("invalid URL: %s", authURL)
+				}
+			} else {
+				return err
+			}
+
 			if !isInteractive {
-				fmt.Fprintf(w, "%s to continue in your web browser: %s\n", cs.Bold("Open this URL"), url)
+				fmt.Fprintf(w, "%s to continue in your web browser: %s\n", cs.Bold("Open this URL"), authURL)
 				return nil
 			}
 
 			fmt.Fprintf(w, "%s to open %s in your browser... ", cs.Bold("Press Enter"), oauthHost)
 			_ = waitForEnter(IO.In)
 
-			// FIXME: read the browser from cmd Factory rather than recreating it
-			browser := cmdutil.NewBrowser(os.Getenv("BROWSER"), IO.Out, IO.ErrOut)
-			if err := browser.Browse(url); err != nil {
-				fmt.Fprintf(w, "%s Failed opening a web browser at %s\n", cs.Red("!"), url)
+			browser := cmdutil.NewBrowser(browserLauncher, IO.Out, IO.ErrOut)
+			if err := browser.Browse(authURL); err != nil {
+				fmt.Fprintf(w, "%s Failed opening a web browser at %s\n", cs.Red("!"), authURL)
 				fmt.Fprintf(w, "  %s\n", err)
 				fmt.Fprint(w, "  Please try entering the URL in your browser manually\n")
 			}
 			return nil
-		},
-		WriteSuccessHTML: func(w io.Writer) {
+		}, WriteSuccessHTML: func(w io.Writer) {
 			fmt.Fprint(w, oauthSuccessPage)
 		},
 		HTTPClient: httpClient,

internal/featuredetection/feature_detection.go

@@ -48,7 +48,6 @@ type detector struct {
 }
 
 func NewDetector(httpClient *http.Client, host string) Detector {
-	// cachedClient := api.NewCachedClient(httpClient, time.Hour*48)
 	return &detector{
 		httpClient: httpClient,
 		host:       host,

pkg/cmd/factory/http.go

@@ -27,7 +27,9 @@ type HTTPClientOptions struct {
 }
 
 func NewHTTPClient(opts HTTPClientOptions) (*http.Client, error) {
-	clientOpts := &ghAPI.ClientOptions{}
+	// Provide default invalid token so gh.HTTPClient will not automatically resolve it.
+	// The real token is inserted at request time with AddAuthTokenHeader.
+	clientOpts := &ghAPI.ClientOptions{AuthToken: "none"}
 
 	if debugEnabled, _ := utils.IsDebugEnabled(); debugEnabled {
 		clientOpts.Log = opts.Log

pkg/cmd/factory/http_test.go

@@ -181,6 +181,7 @@ func TestNewHTTPClient(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+
 			oldDebug := os.Getenv("DEBUG")
 			oldGhDebug := os.Getenv("GH_DEBUG")
 			os.Setenv("DEBUG", tt.envDebug)
@@ -213,6 +214,7 @@ func TestNewHTTPClient(t *testing.T) {
 			require.NoError(t, err)
 
 			res, err := client.Do(req)
+
 			require.NoError(t, err)
 
 			for name, value := range tt.wantHeader {

pkg/cmd/issue/create/create.go

@@ -155,7 +155,7 @@ func createRun(opts *CreateOptions) (err error) {
 		}
 	}
 
-	tpl := shared.NewTemplateManager(httpClient, baseRepo, opts.RootDirOverride, !opts.HasRepoOverride, false)
+	tpl := shared.NewTemplateManager(httpClient, nil, baseRepo, opts.RootDirOverride, !opts.HasRepoOverride, false)
 
 	if opts.WebMode {
 		var openURL string

pkg/cmd/pr/create/create.go

@@ -287,14 +287,14 @@ func createRun(opts *CreateOptions) (err error) {
 	if !opts.BodyProvided {
 		templateContent := ""
 		if opts.RecoverFile == "" {
-			// var cachedClient *http.Client
-			// cachedClient, err = opts.CachedHttpClient(time.Hour * 24)
-			// if err != nil {
-			// return
-			// }
-
-			tpl := shared.NewTemplateManager(client.HTTP(), ctx.BaseRepo, opts.RootDirOverride, opts.RepoOverride == "", true)
-			// tpl.SetCachedClient(cachedClient)
+			var cachedClient *http.Client
+			cachedClient, err = opts.CachedHttpClient(time.Hour * 24)
+			if err != nil {
+				return
+			}
+
+			tpl := shared.NewTemplateManager(client.HTTP(), cachedClient, ctx.BaseRepo, opts.RootDirOverride, opts.RepoOverride == "", true)
+
 			var template shared.Template
 			template, err = tpl.Choose()
 			if err != nil {

pkg/cmd/pr/shared/templates.go

@@ -129,14 +129,14 @@ type templateManager struct {
 	fetchError error
 }
 
-func NewTemplateManager(httpClient *http.Client, repo ghrepo.Interface, dir string, allowFS bool, isPR bool) *templateManager {
+func NewTemplateManager(httpClient *http.Client, cachedClient *http.Client, repo ghrepo.Interface, dir string, allowFS bool, isPR bool) *templateManager {
 	return &templateManager{
 		repo:       repo,
 		rootDir:    dir,
 		allowFS:    allowFS,
 		isPR:       isPR,
 		httpClient: httpClient,
-		detector:   fd.NewDetector(httpClient, repo.RepoHost()),
+		detector:   fd.NewDetector(cachedClient, repo.RepoHost()),
 	}
 }