Skip to content

Commit

Permalink
cover most common error (#197)
Browse files Browse the repository at this point in the history
* cover most common error

* Error getting admin callback, got

* additional error handling

* use tflog for retry message

* add log

* another retryable section

* parallel tests

* clean up logs, prep for merge

* update for next version build

* update to run tests in parallel by default

* downgrade go to match pingone
  • Loading branch information
samir-gandhi authored Dec 4, 2023
1 parent 9b707e8 commit 1d20289
Show file tree
Hide file tree
Showing 17 changed files with 270 additions and 140 deletions.
4 changes: 2 additions & 2 deletions GNUmakefile
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ test:

testacc:
@echo "==> Running acceptance tests..."
TF_ACC=1 go test $(TEST) -v $(TESTARGS) -timeout 120m
TF_ACC=1 go test $(TEST) -v $(TESTARGS) -timeout 120m -parallel 15

sweep:
@echo "==> Running sweep..."
Expand Down Expand Up @@ -101,4 +101,4 @@ terrafmtcheck:

devcheck: build vet tools generate terrafmt docscategorycheck lint test sweep testacc

.PHONY: tools build generate docscategorycheck test testacc sweep vet fmtcheck depscheck lint golangci-lint importlint providerlint tflint terrafmt terrafmtcheck
.PHONY: tools build generate docscategorycheck test testacc sweep vet fmtcheck depscheck lint golangci-lint importlint providerlint tflint terrafmt terrafmtcheck testacc devcheck
34 changes: 34 additions & 0 deletions docs/resources/connection.md
Original file line number Diff line number Diff line change
Expand Up @@ -1270,6 +1270,23 @@ If the `value` type of a property is not defined it must be inferred.



### HYPR Adapt

**Connector Display Name**: HYPR Adapt

**Connector ID** - schema `connectorId`: connectorHyprAdapt

**Properties Table:**



| Display Name | `name` | `value` Type | Description |
| ---- | ---- | ---- | ----|
| HYPR Adapt Access Token | `accessToken` | `string` | Access Token |




### Have I Been Pwned

**Connector Display Name**: Have I Been Pwned
Expand Down Expand Up @@ -1432,6 +1449,23 @@ If the `value` type of a property is not defined it must be inferred.



### IDmelon

**Connector Display Name**: IDmelon

**Connector ID** - schema `connectorId`: connectorIdmelon

**Properties Table:**



| Display Name | `name` | `value` Type | Description |
| ---- | ---- | ---- | ----|
| Custom Parameters | `customAuth` | `array` | |




### IDmission

**Connector Display Name**: IDmission
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ require (
github.com/patrickcping/pingone-go-sdk-v2/management v0.32.0
github.com/patrickcping/pingone-go-sdk-v2/mfa v0.18.0
github.com/pavius/impi v0.0.3
github.com/samir-gandhi/davinci-client-go v0.0.54
github.com/samir-gandhi/davinci-client-go v0.0.55
github.com/samir-gandhi/dvgenerate v0.0.7
github.com/terraform-linters/tflint v0.48.0
golang.org/x/exp v0.0.0-20231108232855-2478ac86f678
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -886,8 +886,8 @@ github.com/ryancurrah/gomodguard v1.3.0 h1:q15RT/pd6UggBXVBuLps8BXRvl5GPBcwVA7BJ
github.com/ryancurrah/gomodguard v1.3.0/go.mod h1:ggBxb3luypPEzqVtq33ee7YSN35V28XeGnid8dnni50=
github.com/ryanrolds/sqlclosecheck v0.4.0 h1:i8SX60Rppc1wRuyQjMciLqIzV3xnoHB7/tXbr6RGYNI=
github.com/ryanrolds/sqlclosecheck v0.4.0/go.mod h1:TBRRjzL31JONc9i4XMinicuo+s+E8yKZ5FN8X3G6CKQ=
github.com/samir-gandhi/davinci-client-go v0.0.54 h1:i7XMXuKii8b0fxyOQra4652oCuKzXOMp/sFmz7Bvh9o=
github.com/samir-gandhi/davinci-client-go v0.0.54/go.mod h1:2Cd34SQC6bFB8zGwWs4Sf3LOb2oNZtpRsxm6w6wrNbE=
github.com/samir-gandhi/davinci-client-go v0.0.55 h1:GuMUWsblO0UbBKxJ21KjL0BZmpuIQekxr8Is0tkpB9w=
github.com/samir-gandhi/davinci-client-go v0.0.55/go.mod h1:2Cd34SQC6bFB8zGwWs4Sf3LOb2oNZtpRsxm6w6wrNbE=
github.com/samir-gandhi/dvgenerate v0.0.7 h1:1ff58iR9pbaYAZ37CTmQW9AgCjHQlMew+JXv2DNMHJ4=
github.com/samir-gandhi/dvgenerate v0.0.7/go.mod h1:MSAcNGRG30YUfSry4JJMZn8GN9Au7RviqnWbaFMxbxQ=
github.com/sanposhiho/wastedassign/v2 v2.0.7 h1:J+6nrY4VW+gC9xFzUc+XjPD3g3wF3je/NsJFwFK7Uxc=
Expand Down
2 changes: 2 additions & 0 deletions internal/acctest/acctest.go
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ func TestClient() (*dv.APIClient, error) {
if environment_id == "" {
e = e + "PINGONE_ENVIRONMENT_ID "
}
host_url := os.Getenv("PINGONE_DAVINCI_HOST_URL")
if e != "" {
return nil, fmt.Errorf("missing environment variables: %s", e)
}
Expand All @@ -395,6 +396,7 @@ func TestClient() (*dv.APIClient, error) {
Password: password,
PingOneRegion: region,
PingOneSSOEnvId: environment_id,
HostURL: host_url,
}
c, err := dv.NewClient(&cInput)
if err != nil {
Expand Down
35 changes: 34 additions & 1 deletion internal/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"strings"

"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
Expand Down Expand Up @@ -121,8 +122,10 @@ func configure(version string, p *schema.Provider) func(context.Context, *schema
AccessToken: accessToken,
UserAgent: fmt.Sprintf("terraform-provider-davinci/%s/go", version),
}
c, err := client.NewClient(&cInput)
c, err := retryableClient(&cInput)
if err != nil {
// DELETE ME
tflog.Info(ctx, "Error initializing client")
return nil, diag.FromErr(err)
}
if environment_id != "" {
Expand All @@ -136,3 +139,33 @@ func configure(version string, p *schema.Provider) func(context.Context, *schema
return c, diags
}
}

func retryableClient(cInput *client.ClientInput) (*client.APIClient, error) {
var c *client.APIClient
var err error
ctx := context.Background()

for retries := 0; retries <= 2; retries++ {
c, err = client.NewClient(cInput)
if retries == 2 && err != nil {
return nil, err
}
switch {
case err == nil:
return c, nil
// These cases come from the davinci-client-go library and may be subject to change
case strings.Contains(err.Error(), "Error getting admin callback, got: status: 502, body:"):
tflog.Info(ctx, "Found retryable error while initializing client. Retrying...")
fmt.Printf("Sign in retryable Error: %s\n", err.Error())
case strings.Contains(err.Error(), "Error getting SSO callback, got err: status: 502, body:"):
tflog.Info(ctx, "Found retryable error while initializing client. Retrying...")
fmt.Printf("Sign in retryable Error: %s\n", err.Error())
case strings.Contains(err.Error(), "Auth Token not found, unsuccessful login, got: Found. Redirecting to https://console.pingone.com/davinci/index.html#/sso/callback/?error=AuthenticationFailed&error_description=unknownError2"):
tflog.Info(ctx, "Found retryable error while initializing client. Retrying...")
fmt.Printf("Sign in retryable Error: %s\n", err.Error())
default:
return nil, err
}
}
return nil, fmt.Errorf("Error initializing client. Please report this as a bug.")
}
80 changes: 62 additions & 18 deletions internal/sdk/retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package sdk
import (
"context"
"fmt"
// "strings"
"time"

"github.com/hashicorp/terraform-plugin-log/tflog"
Expand Down Expand Up @@ -41,34 +42,77 @@ func DoRetryableError(ctx context.Context, f func() (interface{}, error), timeou
}
return res, err
}
if timeElapsed >= *timeout {
return nil, fmt.Errorf("Operation was not successful within timeout.")
}
return nil, fmt.Errorf("Operation was not successful within timeout.")
}

// RetryableError is an error that can be retried
// DoRetryable wraps the given function with a rery loop that doubles the timeout each time.
// This is used to catch and handle common errors that may be due to rate limiting or an issue in the API.
// Error functions in this list should be safe to retry and have a TODO to remove them if the issue is resolved.
func DoRetryable(ctx context.Context, f func() (interface{}, error), timeout *time.Duration) (interface{}, error) {
defaultTimeout := 20 * time.Second
timeElapsed := 0 * time.Second
if timeout == nil {
timeout = &defaultTimeout
defaultTimeout := 20
timeoutMax := defaultTimeout
timeElapsed := 1
var err error
if timeout != nil {
timeoutMax = int(*timeout)
}
for timeElapsed < *timeout {
for timeElapsed < timeoutMax {
if timeElapsed > 1 {
fmt.Println("Sleeping for ", time.Duration(timeElapsed)*time.Second, " seconds")
time.Sleep(time.Duration(timeElapsed) * time.Second)
}
res, err := f()
if err != nil {
dvErr, err := dv.ParseDvHttpError(err)
if err != nil {
return nil, err
}
// TODO: Handle other error codes as they are discovered
if dvErr.Status == 502 && dvErr.Body == "" {
if err == nil {
return res, err
}
dvErr, parseErr := dv.ParseDvHttpError(err)
if parseErr != nil {
return nil, err
}
// TODO: Handle other error codes as they are discovered
switch dvErr.Status {
case 502:
switch dvErr.Body {
case "":
tflog.Info(ctx, "Rate limit hit, retrying...")
timeElapsed += 2 * time.Second
time.Sleep(2 * time.Second)
continue
fmt.Println("Rate limit hit, retrying...")
default:
return nil, err
}
// case 400:
// switch {
// case strings.Contains(dvErr.Body, fmt.Sprintf(`"cause":null,"logLevel":"error","serviceName":null,"message":"Error deleting flow","errorMessage":"Error deleting flow","success":false,"httpResponseCode":400,"code":8032`)):
// tflog.Info(ctx, "Retryable error on connections endpoint, retrying...")
// fmt.Println("Retryable error on connections endpoint: Error deleting flow, retrying...")
// case strings.Contains(dvErr.Body, fmt.Sprintf(`"cause":null,"logLevel":"error","serviceName":null,"message":"Error deleting record","errorMessage":"Error deleting record","success":false,"httpResponseCode":400,"code":6008`)):
// tflog.Info(ctx, "Retryable error while deleting record, retrying...")
// fmt.Println("Retryable error while deleting record, retrying...")
// case strings.Contains(dvErr.Body, fmt.Sprintf(`"cause":null,"logLevel":"error","serviceName":null,"message":"Error updating flow","errorMessage":"Error updating flow","success":false,"httpResponseCode":400,"code":8033`)):
// tflog.Info(ctx, "Retryable error while updating flow, retrying...")
// fmt.Println("Retryable error while updating flow, retrying...")
// // This error occurs occasionally, but is not retryable because it is the same error when reading a connection that does not exist.
// // case strings.Contains(dvErr.Body, fmt.Sprintf(`"cause":null,"logLevel":"error","serviceName":null,"message":"Error retrieving connectors","errorMessage":"Error retrieving connectors","success":false,"httpResponseCode":400,"code":7005`)):
// // tflog.Info(ctx, "Retryable error on connections endpoint, retrying...")
// // fmt.Println("Retryable error on connections endpoint: Error retrieving connectors, retrying...")
// // This error occurs occasionally, but is not retryable because it is the same error when deleting a connection that does not exist.
// // case strings.Contains(dvErr.Body, fmt.Sprintf(`"cause":null,"logLevel":"error","serviceName":null,"message":"Connectors not found","errorMessage":"Connectors not found","success":false,"httpResponseCode":400,"code":7006`)):
// // tflog.Info(ctx, "Retryable error on connections endpoint, retrying...")
// // fmt.Println("Retryable error on connections endpoint: Connectors not found, retrying...")
// default:
// return nil, err
// }
default:
return nil, err
}
return res, err
timeElapsed = timeElapsed * 2
continue
}
return nil, fmt.Errorf("Operation was not successful within timeout.")
//remove this line
// panic("Operation was not successful within timeout.")
return nil, err
}

type RetryableError struct {
Expand Down
32 changes: 30 additions & 2 deletions internal/sdk/sdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func CheckAndRefreshAuth(ctx context.Context, c *dv.APIClient, d *schema.Resourc
return nil
}
timeout := 100
freshEnvTimeout := 50
freshEnvTimeout := 60
for i := 0; i <= timeout; {
// initially, test if auth is valid for the target environment
apps, err := c.ReadApplications(&envId, nil)
Expand All @@ -51,7 +51,7 @@ func CheckAndRefreshAuth(ctx context.Context, c *dv.APIClient, d *schema.Resourc
tflog.Warn(ctx, "Possible fresh DaVinci env. Retrying Auth ... ")
}
}
err = c.InitAuth()
err = initAuthRetryable(ctx, c)
if err != nil {
return err
}
Expand All @@ -76,3 +76,31 @@ func CheckAndRefreshAuth(ctx context.Context, c *dv.APIClient, d *schema.Resourc
}
return nil
}

// re-initialize auth, with optionial retry
func initAuthRetryable(ctx context.Context, c *dv.APIClient) error {
for retries := 0; retries <= 3; retries++ {
err := c.InitAuth()
if retries == 3 && err != nil {
return err
}
switch {
case err == nil:
return nil
// These cases come from the davinci-client-go library and may be subject to change
case strings.Contains(err.Error(), "Error getting admin callback, got: status: 502, body:"):
tflog.Info(ctx, "Found retryable error while initializing client. Retrying...")
fmt.Printf("Sign in retryable Error: %s\n", err.Error())
case strings.Contains(err.Error(), "Error getting SSO callback, got err: status: 502, body:"):
tflog.Info(ctx, "Found retryable error while initializing client. Retrying...")
fmt.Printf("Sign in retryable Error: %s\n", err.Error())
case strings.Contains(err.Error(), "Auth Token not found, unsuccessful login, got: Found. Redirecting to https://console.pingone.com/davinci/index.html#/sso/callback/?error=AuthenticationFailed&error_description=unknownError2"):
tflog.Info(ctx, "Found retryable error while initializing client. Retrying...")
fmt.Printf("Sign in retryable Error: %s\n", err.Error())
default:
tflog.Info(ctx, "Error re-initializing authorization.")
return err
}
}
return fmt.Errorf("Error re-initializing authorization. Please report this as a bug.")
}
10 changes: 7 additions & 3 deletions internal/service/davinci/data_source_application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,24 @@ package davinci_test
import (
"fmt"
"testing"
"time"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/pingidentity/terraform-provider-davinci/internal/acctest"
)

// TestAccDatasourceApplication_Slim - Depends on testAccResourceApplication_Slim_Hcl
func TestAccDatasourceApplication_SlimByAppId(t *testing.T) {
// tflog.Info(ctx, "Rate limit hit, retrying...")
timeElapsed := 4 * time.Second
fmt.Println(timeElapsed)
fmt.Println(timeElapsed)

resourceBase := "davinci_application"
resourceName := acctest.ResourceNameGen()
dataSourceFullName := fmt.Sprintf("data.%s.%s", resourceBase, resourceName)
hcl := testAccDatasourceApplication_SlimByAppId_Hcl(resourceName)

resource.Test(t, resource.TestCase{
resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheckPingOneAndTfVars(t) },
ProviderFactories: acctest.ProviderFactories,
ExternalProviders: acctest.ExternalProviders,
Expand All @@ -41,7 +45,7 @@ func TestAccDatasourceApplication_SlimById(t *testing.T) {
dataSourceFullName := fmt.Sprintf("data.%s.%s", resourceBase, resourceName)
hcl := testAccDatasourceApplication_SlimById_Hcl(resourceName)

resource.Test(t, resource.TestCase{
resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheckPingOneAndTfVars(t) },
ProviderFactories: acctest.ProviderFactories,
ExternalProviders: acctest.ExternalProviders,
Expand Down
2 changes: 1 addition & 1 deletion internal/service/davinci/data_source_applications_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func TestAccDataSourceApplications_AllApplications(t *testing.T) {

hcl := testAccDataSourceApplications_AllApplications_Hcl(resourceName)

resource.Test(t, resource.TestCase{
resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheckPingOneAndTfVars(t) },
ProviderFactories: acctest.ProviderFactories,
ExternalProviders: acctest.ExternalProviders,
Expand Down
Loading

0 comments on commit 1d20289

Please sign in to comment.