registry: Fix panic when server is unreachable

Non-HTTP errors previously resulted in a panic due to dereferencing the
resp pointer while it was nil, as part of rendering the error message.
This commit changes the error message formatting to cope with a nil
response, and extends test coverage.

Fixes #24384
This commit is contained in:
Alisdair McDiarmid 2020-03-19 10:20:10 -04:00
parent 7d6d653b4f
commit 1c1df6dc50
3 changed files with 70 additions and 7 deletions

View File

@ -14,7 +14,7 @@ import (
"time" "time"
"github.com/hashicorp/go-retryablehttp" "github.com/hashicorp/go-retryablehttp"
"github.com/hashicorp/terraform-svchost" svchost "github.com/hashicorp/terraform-svchost"
"github.com/hashicorp/terraform-svchost/disco" "github.com/hashicorp/terraform-svchost/disco"
"github.com/hashicorp/terraform/helper/logging" "github.com/hashicorp/terraform/helper/logging"
"github.com/hashicorp/terraform/httpclient" "github.com/hashicorp/terraform/httpclient"
@ -413,15 +413,23 @@ func maxRetryErrorHandler(resp *http.Response, err error, numTries int) (*http.R
resp.Body.Close() resp.Body.Close()
} }
// Additional error detail: if we have a response, use the status code;
// if we have an error, use that; otherwise nothing. We will never have
// both response and error.
var errMsg string var errMsg string
if err != nil { if resp != nil {
errMsg = fmt.Sprintf(" %s", err) errMsg = fmt.Sprintf(": %d", resp.StatusCode)
} else if err != nil {
errMsg = fmt.Sprintf(": %s", err)
} }
// This function is always called with numTries=RetryMax+1. If we made any
// retry attempts, include that in the error message.
if numTries > 1 { if numTries > 1 {
return resp, fmt.Errorf("the request failed after %d attempts, please try again later: %d%s", return resp, fmt.Errorf("the request failed after %d attempts, please try again later%s",
numTries, resp.StatusCode, errMsg) numTries, errMsg)
} }
return resp, fmt.Errorf("the request failed, please try again later: %d%s", resp.StatusCode, errMsg) return resp, fmt.Errorf("the request failed, please try again later%s", errMsg)
} }
// configureRequestTimeout configures the registry client request timeout from // configureRequestTimeout configures the registry client request timeout from

View File

@ -314,6 +314,61 @@ func TestLookupModuleRetryError(t *testing.T) {
} }
} }
func TestLookupModuleNoRetryError(t *testing.T) {
// Disable retries
discoveryRetry = 0
defer configureDiscoveryRetry()
server := test.RegistryRetryableErrorsServer()
defer server.Close()
client := NewClient(test.Disco(server), nil)
src := "example.com/test-versions/name/provider"
modsrc, err := regsrc.ParseModuleSource(src)
if err != nil {
t.Fatal(err)
}
resp, err := client.ModuleVersions(modsrc)
if err == nil {
t.Fatal("expected request to fail", err)
}
if resp != nil {
t.Fatal("unexpected response", *resp)
}
// verify maxRetryErrorHandler handler returned the error
if !strings.Contains(err.Error(), "the request failed, please try again later") {
t.Fatal("unexpected error, got:", err)
}
}
func TestLookupModuleNetworkError(t *testing.T) {
server := test.RegistryRetryableErrorsServer()
client := NewClient(test.Disco(server), nil)
// Shut down the server to simulate network failure
server.Close()
src := "example.com/test-versions/name/provider"
modsrc, err := regsrc.ParseModuleSource(src)
if err != nil {
t.Fatal(err)
}
resp, err := client.ModuleVersions(modsrc)
if err == nil {
t.Fatal("expected request to fail", err)
}
if resp != nil {
t.Fatal("unexpected response", *resp)
}
// verify maxRetryErrorHandler handler returned the correct error
if !strings.Contains(err.Error(), "the request failed after 2 attempts, please try again later") {
t.Fatal("unexpected error, got:", err)
}
}
func TestLookupProviderVersions(t *testing.T) { func TestLookupProviderVersions(t *testing.T) {
server := test.Registry() server := test.Registry()
defer server.Close() defer server.Close()

View File

@ -12,7 +12,7 @@ import (
"strings" "strings"
version "github.com/hashicorp/go-version" version "github.com/hashicorp/go-version"
"github.com/hashicorp/terraform-svchost" svchost "github.com/hashicorp/terraform-svchost"
"github.com/hashicorp/terraform-svchost/auth" "github.com/hashicorp/terraform-svchost/auth"
"github.com/hashicorp/terraform-svchost/disco" "github.com/hashicorp/terraform-svchost/disco"
"github.com/hashicorp/terraform/httpclient" "github.com/hashicorp/terraform/httpclient"