Optimize Branch Cli error output (#109)

* test error * fix * add check interval * fix * fix * fix * update timeout * optimize interval
tidbcloud · Jul 17, 2023 · b4878c3 · b4878c3
1 parent 4ab586e
commit b4878c3
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 21 deletions.
diff --git a/internal/cli/branch/create.go b/internal/cli/branch/create.go
@@ -40,7 +40,7 @@ var createBranchField = map[string]int{
 }
 
 const (
-	WaitInterval = 1 * time.Second
+	WaitInterval = 5 * time.Second
 	WaitTimeout  = 5 * time.Minute
 )
 

diff --git a/internal/cli/cluster/create.go b/internal/cli/cluster/create.go
@@ -46,6 +46,8 @@ const (
 const (
 	serverlessType = "SERVERLESS"
 	developerType  = "DEVELOPER"
+	WaitInterval   = 5 * time.Second
+	WaitTimeout    = 2 * time.Minute
 )
 
 type CreateOpts struct {
@@ -303,9 +305,9 @@ func CreateAndWaitReady(h *internal.Helper, d cloud.TiDBCloudClient, projectID s
 	newClusterID := *createClusterResult.GetPayload().ID
 
 	fmt.Fprintln(h.IOStreams.Out, "... Waiting for cluster to be ready")
-	ticker := time.NewTicker(1 * time.Second)
+	ticker := time.NewTicker(WaitInterval)
 	defer ticker.Stop()
-	timer := time.After(2 * time.Minute)
+	timer := time.After(WaitTimeout)
 	for {
 		select {
 		case <-timer:
@@ -335,9 +337,9 @@ func CreateAndSpinnerWait(ctx context.Context, d cloud.TiDBCloudClient, projectI
 		}
 		newClusterID := *createClusterResult.GetPayload().ID
 
-		ticker := time.NewTicker(1 * time.Second)
+		ticker := time.NewTicker(WaitInterval)
 		defer ticker.Stop()
-		timer := time.After(2 * time.Minute)
+		timer := time.After(WaitTimeout)
 		for {
 			select {
 			case <-timer:

diff --git a/internal/service/cloud/api_client.go b/internal/service/cloud/api_client.go
@@ -168,37 +168,25 @@ func (d *ClientDelegate) GetConnectInfo(params *connectInfoOp.GetInfoParams, opt
 
 func (d *ClientDelegate) GetBranch(params *branchOp.GetBranchParams, opts ...branchOp.ClientOption) (*branchOp.GetBranchOK, error) {
 	r, err := d.bc.BranchService.GetBranch(params, opts...)
-	if err != nil {
-		errorPayload := err.(*branchOp.GetBranchDefault).Payload.Error
-		return nil, fmt.Errorf("[GET /api/v1beta/clusters/{cluster_id}/branches/{branch_id}][%d] GetBranch  %+v", errorPayload.Code, errorPayload.Message)
-	}
+	err = parseBranchError(err)
 	return r, err
 }
 
 func (d *ClientDelegate) ListBranches(params *branchOp.ListBranchesParams, opts ...branchOp.ClientOption) (*branchOp.ListBranchesOK, error) {
 	r, err := d.bc.BranchService.ListBranches(params, opts...)
-	if err != nil {
-		errorPayload := err.(*branchOp.ListBranchesDefault).Payload.Error
-		return nil, fmt.Errorf("[GET /api/v1beta/clusters/{cluster_id}/branches][%d] ListBranches  %+v", errorPayload.Code, errorPayload.Message)
-	}
+	err = parseBranchError(err)
 	return r, err
 }
 
 func (d *ClientDelegate) CreateBranch(params *branchOp.CreateBranchParams, opts ...branchOp.ClientOption) (*branchOp.CreateBranchOK, error) {
 	r, err := d.bc.BranchService.CreateBranch(params, opts...)
-	if err != nil {
-		errorPayload := err.(*branchOp.CreateBranchDefault).Payload.Error
-		return nil, fmt.Errorf("[POST /api/v1beta/clusters/{cluster_id}/branches][%d] CreateBranch  %+v", errorPayload.Code, errorPayload.Message)
-	}
+	err = parseBranchError(err)
 	return r, err
 }
 
 func (d *ClientDelegate) DeleteBranch(params *branchOp.DeleteBranchParams, opts ...branchOp.ClientOption) (*branchOp.DeleteBranchOK, error) {
 	r, err := d.bc.BranchService.DeleteBranch(params, opts...)
-	if err != nil {
-		errorPayload := err.(*branchOp.DeleteBranchDefault).Payload.Error
-		return nil, fmt.Errorf("[DELETE /api/v1beta/clusters/{cluster_id}/branches/{branch_id}][%d] DeleteBranch  %+v", errorPayload.Code, errorPayload.Message)
-	}
+	err = parseBranchError(err)
 	return r, err
 }
 
@@ -238,3 +226,46 @@ func (ug *UserAgentTransport) RoundTrip(r *http.Request) (*http.Response, error)
 	r.Header.Set(userAgent, ug.Agent)
 	return ug.inner.RoundTrip(r)
 }
+
+func parseBranchError(err error) error {
+	if err == nil {
+		return nil
+	}
+
+	switch e := err.(type) {
+	case *branchOp.DeleteBranchDefault:
+		msg := "[DELETE /api/v1beta/clusters/{cluster_id}/branches/{branch_id}] DeleteBranch"
+		// return by api gateway
+		if e.Payload == nil || e.Payload.Error == nil {
+			return fmt.Errorf(msg+" [%d] unknown error", e.Code())
+		}
+		// return by serverless-svc
+		return fmt.Errorf(msg+" [%d] %+v", e.Payload.Error.Code, e.Payload.Error.Message)
+	case *branchOp.GetBranchDefault:
+		msg := "[GET /api/v1beta/clusters/{cluster_id}/branches/{branch_id}] GetBranch"
+		// return by api gateway
+		if e.Payload == nil || e.Payload.Error == nil {
+			return fmt.Errorf(msg+" [%d] unknown error", e.Code())
+		}
+		// return by serverless-svc
+		return fmt.Errorf(msg+" [%d] %+v", e.Payload.Error.Code, e.Payload.Error.Message)
+	case *branchOp.ListBranchesDefault:
+		msg := "[GET /api/v1beta/clusters/{cluster_id}/branches] ListBranches"
+		// return by api gateway
+		if e.Payload == nil || e.Payload.Error == nil {
+			return fmt.Errorf(msg+" [%d] unknown error", e.Code())
+		}
+		// return by serverless-svc
+		return fmt.Errorf(msg+" [%d] %+v", e.Payload.Error.Code, e.Payload.Error.Message)
+	case *branchOp.CreateBranchDefault:
+		msg := "[POST /api/v1beta/clusters/{cluster_id}/branches] CreateBranch"
+		// return by api gateway
+		if e.Payload == nil || e.Payload.Error == nil {
+			return fmt.Errorf(msg+" [%d] unknown error", e.Code())
+		}
+		// return by serverless-svc
+		return fmt.Errorf(msg+" [%d] %+v", e.Payload.Error.Code, e.Payload.Error.Message)
+	default:
+		return err
+	}
+}