diff --git a/cmd/execute/contract_call.go b/cmd/execute/contract_call.go index 6ea1f54..4fb582a 100644 --- a/cmd/execute/contract_call.go +++ b/cmd/execute/contract_call.go @@ -125,6 +125,9 @@ func NewContractCallCmd(f *cmdutil.Factory) *cobra.Command { } if execTerminalStatuses[writeResp.Status] { + if err := terminalExecError(writeResp.ExecutionID, writeResp.Status, nil); err != nil { + return err + } return printContractCallWriteResult(p, &writeResp) } diff --git a/cmd/execute/contract_call_test.go b/cmd/execute/contract_call_test.go index 4a3f00b..b946885 100644 --- a/cmd/execute/contract_call_test.go +++ b/cmd/execute/contract_call_test.go @@ -303,3 +303,33 @@ func TestContractCallCmd_WaitWritePolls(t *testing.T) { t.Errorf("expected tx hash in output, got: %q", out) } } + +func TestContractCallCmd_WaitFailsWhenWriteResponseAlreadyFailed(t *testing.T) { + pollCount := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if strings.HasSuffix(r.URL.Path, "/status") { + pollCount++ + } + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"executionId":"exec-ccfail","status":"failed"}`)) + })) + defer srv.Close() + + ios, _, _, _ := iostreams.Test() + f := newContractCallFactory(ios, srv) + + cmd := execute.NewContractCallCmd(f) + cmd.SetArgs([]string{"--chain", "1", "--contract", "0xcontract", "--method", "transfer", "--wait", "--timeout", "10s"}) + + err := cmd.Execute() + if err == nil { + t.Fatal("expected an error when the write response is already failed, got nil") + } + if !strings.Contains(err.Error(), "exec-ccfail") { + t.Errorf("expected the execution id in the error, got: %q", err.Error()) + } + if pollCount > 0 { + t.Errorf("expected no polling when already terminal, got %d polls", pollCount) + } +} diff --git a/cmd/execute/transfer.go b/cmd/execute/transfer.go index 04f7589..84c2e91 100644 --- a/cmd/execute/transfer.go +++ b/cmd/execute/transfer.go @@ -32,6 +32,24 @@ var execTerminalStatuses = map[string]bool{ "failed": true, } +// terminalExecError reports a terminal status that did not succeed. +// +// Two paths reach a terminal status: the write response can already carry one, +// and pollExecStatus reads one from the status endpoint. Both must classify it +// the same way. They did not, so a write that failed fast exited zero while the +// identical failure discovered one poll later exited non-zero. +// +// apiErr is nil on the write path, whose response carries no error detail. +func terminalExecError(executionID, status string, apiErr *string) error { + if status != "failed" { + return nil + } + if apiErr != nil && *apiErr != "" { + return fmt.Errorf("%s", *apiErr) + } + return fmt.Errorf("execution %s failed", executionID) +} + func NewTransferCmd(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "transfer", @@ -110,6 +128,9 @@ func NewTransferCmd(f *cmdutil.Factory) *cobra.Command { } if execTerminalStatuses[execResp.Status] { + if err := terminalExecError(execResp.ExecutionID, execResp.Status, nil); err != nil { + return err + } return printTransferResult(p, &execResp) } @@ -157,12 +178,8 @@ func pollExecStatus(f *cmdutil.Factory, client *khhttp.Client, host, executionID } if execTerminalStatuses[statusResp.Status] { - if statusResp.Status == "failed" { - msg := fmt.Sprintf("execution %s failed", executionID) - if statusResp.Error != nil { - msg = *statusResp.Error - } - return fmt.Errorf("%s", msg) + if err := terminalExecError(executionID, statusResp.Status, statusResp.Error); err != nil { + return err } return printExecStatusResult(p, statusResp) } diff --git a/cmd/execute/transfer_test.go b/cmd/execute/transfer_test.go index 2fcfc0a..ee05786 100644 --- a/cmd/execute/transfer_test.go +++ b/cmd/execute/transfer_test.go @@ -237,3 +237,67 @@ func TestTransferCmd_WaitPolls(t *testing.T) { } } +func TestTransferCmd_WaitFailsWhenWriteResponseAlreadyFailed(t *testing.T) { + pollCount := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/status") { + pollCount++ + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"executionId":"exec-fastfail","status":"failed"}`)) + })) + defer srv.Close() + + ios, _, _, _ := iostreams.Test() + f := newTransferFactory(ios, srv) + + cmd := execute.NewTransferCmd(f) + cmd.SetArgs([]string{"--chain", "1", "--to", "0xabc", "--amount", "0.1", "--wait"}) + + err := cmd.Execute() + if err == nil { + t.Fatal("expected an error when the write response is already failed, got nil") + } + if !strings.Contains(err.Error(), "exec-fastfail") { + t.Errorf("expected the execution id in the error, got: %q", err.Error()) + } + if pollCount > 0 { + t.Errorf("expected no polling when already terminal, got %d polls", pollCount) + } +} + +func TestTransferCmd_WaitReportsServerErrorFromStatus(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if strings.HasSuffix(r.URL.Path, "/status") { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"executionId":"exec-slowfail","status":"failed","error":"insufficient funds for gas"}`)) + return + } + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"executionId":"exec-slowfail","status":"pending"}`)) + })) + defer srv.Close() + + ios, _, _, _ := iostreams.Test() + f := newTransferFactory(ios, srv) + + cmd := execute.NewTransferCmd(f) + cmd.SetArgs([]string{"--chain", "1", "--to", "0xabc", "--amount", "0.1", "--wait", "--timeout", "10s"}) + + done := make(chan error, 1) + go func() { done <- cmd.Execute() }() + + select { + case err := <-done: + if err == nil { + t.Fatal("expected an error for a failed execution, got nil") + } + if !strings.Contains(err.Error(), "insufficient funds for gas") { + t.Errorf("expected the server error detail, got: %q", err.Error()) + } + case <-time.After(5 * time.Second): + t.Fatal("command timed out") + } +}