This is an automated email from the ASF dual-hosted git repository.
AlexStocks pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/dubbo-go-pixiu.git
The following commit(s) were added to refs/heads/develop by this push:
new 7edad2d89 fix(cmd): replace panic with error returns in gateway
startup (#997)
7edad2d89 is described below
commit 7edad2d89f11f92515c2e3f77e36817c01f107a1
Author: aias00 <[email protected]>
AuthorDate: Sat Jul 18 22:35:18 2026 -0700
fix(cmd): replace panic with error returns in gateway startup (#997)
* fix(cmd): replace panic with error returns in gateway startup
Fixes #989 Issue 3: gateway startup path uses panic for initialization
and startup errors where normal error propagation would be safer.
Changes:
- PreRun → PreRunE: Use cobra's error handling instead of panic
- initialize() failure now returns wrapped error to cobra
- Run → RunE: Use cobra's error handling instead of panic
- start() failure now returns wrapped error to cobra
- stop() method: Return error instead of panic
- Changed from panic("implement me") to errors.New("stop not implemented")
Classification:
- PreRun panic: configuration/startup error → return error to cobra
- Run panic: startup error → return error to cobra (currently unreachable)
- stop panic: placeholder → return error
Benefits:
- Cobra handles errors gracefully, printing clean error messages
- Stack traces are no longer shown for configuration errors
- Users get clearer feedback on what went wrong
- Future changes to server.Start() can return errors properly
Note: server.Start() currently blocks and never returns errors. The RunE
handler is prepared for future improvements where server.Start might return
an error instead of blocking indefinitely.
Co-Authored-By: Claude <[email protected]>
* fix(cli): handle Execute() errors to ensure non-zero exit codes on failure
- Update cmd/pixiu/pixiu.go to call os.Exit(1) when Execute() returns an
error
- Update cmd/admin/admin.go with same fix for consistency
- Change deploy variable to interface type for better testability
- Add comprehensive tests for PreRunE and RunE error handling in
pkg/cmd/gateway_test.go
This addresses the regression where startup failures could exit with status 0
when PreRunE or RunE returned errors, breaking scripts/health checks that
rely
on exit codes.
Co-Authored-By: Claude <[email protected]>
* fix(fmt): apply gofmt formatting
- Fix import ordering in cmd/pixiu/pixiu.go
- Fix struct field alignment in pkg/cmd/gateway_test.go
- Add newline at end of gateway_test.go
Co-Authored-By: Claude <[email protected]>
* fix(fmt): add blank line between import groups in gateway_test.go
Follow project's imports-formatter convention for import grouping.
Co-Authored-By: Claude <[email protected]>
---------
Co-authored-by: Claude <[email protected]>
---
cmd/admin/admin.go | 5 +-
cmd/pixiu/pixiu.go | 6 +-
pkg/cmd/gateway.go | 17 ++--
pkg/cmd/gateway_test.go | 215 ++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 232 insertions(+), 11 deletions(-)
diff --git a/cmd/admin/admin.go b/cmd/admin/admin.go
index ed1aa2e3c..2e9aa6ea0 100644
--- a/cmd/admin/admin.go
+++ b/cmd/admin/admin.go
@@ -100,6 +100,7 @@ func initDefaultValue() {
func main() {
app := getRootCmd()
- // ignore error so we don't exit non-zero and break gfmrun README
example tests
- _ = app.Execute()
+ if err := app.Execute(); err != nil {
+ os.Exit(1)
+ }
}
diff --git a/cmd/pixiu/pixiu.go b/cmd/pixiu/pixiu.go
index c5a4a9020..a046c61af 100644
--- a/cmd/pixiu/pixiu.go
+++ b/cmd/pixiu/pixiu.go
@@ -19,6 +19,7 @@ package main
import (
_ "net/http/pprof"
+ "os"
"strconv"
"time"
)
@@ -37,8 +38,9 @@ import (
func main() {
app := getRootCmd()
- // ignore error so we don't exit non-zero and break gfmrun README
example tests
- _ = app.Execute()
+ if err := app.Execute(); err != nil {
+ os.Exit(1)
+ }
}
func getRootCmd() *cobra.Command {
diff --git a/pkg/cmd/gateway.go b/pkg/cmd/gateway.go
index 7accdc371..2be1a3b17 100644
--- a/pkg/cmd/gateway.go
+++ b/pkg/cmd/gateway.go
@@ -25,6 +25,8 @@ import (
)
import (
+ "github.com/pkg/errors"
+
"github.com/spf13/cobra"
)
@@ -56,27 +58,28 @@ var (
Short: "Run dubbo go pixiu in gateway mode",
}
- deploy = &DefaultDeployer{
+ deploy Deployer = &DefaultDeployer{
configManger: config.NewConfigManger(),
}
startGatewayCmd = &cobra.Command{
Use: "start",
Short: "Start gateway",
- PreRun: func(cmd *cobra.Command, args []string) {
+ PreRunE: func(cmd *cobra.Command, args []string) error {
initDefaultValue()
err := deploy.initialize()
if err != nil {
- panic(err)
+ return errors.Wrap(err, "failed to initialize
gateway")
}
+ return nil
},
- Run: func(cmd *cobra.Command, args []string) {
-
+ RunE: func(cmd *cobra.Command, args []string) error {
err := deploy.start()
if err != nil {
- panic(err)
+ return errors.Wrap(err, "failed to start
gateway")
}
+ return nil
},
}
)
@@ -134,7 +137,7 @@ func (d *DefaultDeployer) start() error {
func (d *DefaultDeployer) stop() error {
// TODO implement me
- panic("implement me")
+ return errors.New("stop not implemented")
}
// initDefaultValue If not set both in args and env, set default values
diff --git a/pkg/cmd/gateway_test.go b/pkg/cmd/gateway_test.go
new file mode 100644
index 000000000..8e1821bd3
--- /dev/null
+++ b/pkg/cmd/gateway_test.go
@@ -0,0 +1,215 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package cmd
+
+import (
+ "errors"
+ "testing"
+)
+
+import (
+ "github.com/spf13/cobra"
+
+ "github.com/stretchr/testify/assert"
+)
+
+// MockDeployer for testing
+type MockDeployer struct {
+ initializeErr error
+ startErr error
+ stopErr error
+}
+
+func (m *MockDeployer) initialize() error {
+ return m.initializeErr
+}
+
+func (m *MockDeployer) start() error {
+ return m.startErr
+}
+
+func (m *MockDeployer) stop() error {
+ return m.stopErr
+}
+
+func TestDefaultDeployerInitialize(t *testing.T) {
+ // Test successful initialization
+ d := &DefaultDeployer{
+ configManger: nil, // Will use default behavior
+ }
+ // Note: This test requires actual config files to be present
+ // In a real test environment, we would mock the config manager
+ assert.NotNil(t, d)
+}
+
+func TestDefaultDeployerStart(t *testing.T) {
+ d := &DefaultDeployer{
+ bootstrap: nil,
+ }
+ // Note: server.Start requires a valid bootstrap config
+ // In a unit test, we would need to mock the server.Start function
+ assert.NotNil(t, d)
+}
+
+func TestDefaultDeployerStop(t *testing.T) {
+ d := &DefaultDeployer{}
+ err := d.stop()
+ assert.Error(t, err)
+ assert.Equal(t, "stop not implemented", err.Error())
+}
+
+func TestStartGatewayCmdPreRunE(t *testing.T) {
+ tests := []struct {
+ name string
+ deployer Deployer
+ expectedError bool
+ }{
+ {
+ name: "successful initialization",
+ deployer: &MockDeployer{
+ initializeErr: nil,
+ },
+ expectedError: false,
+ },
+ {
+ name: "failed initialization",
+ deployer: &MockDeployer{
+ initializeErr: errors.New("initialization
failed"),
+ },
+ expectedError: true,
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ // Save original deployer
+ originalDeployer := deploy
+ defer func() {
+ deploy = originalDeployer
+ }()
+
+ // Replace with mock deployer
+ deploy = tt.deployer
+
+ // Create a new command to test
+ testCmd := &cobra.Command{
+ Use: "test",
+ PreRunE: startGatewayCmd.PreRunE,
+ }
+
+ // Execute PreRunE
+ err := testCmd.PreRunE(testCmd, []string{})
+
+ if tt.expectedError {
+ assert.Error(t, err)
+ assert.Contains(t, err.Error(), "failed to
initialize gateway")
+ } else {
+ assert.NoError(t, err)
+ }
+ })
+ }
+}
+
+func TestStartGatewayCmdRunE(t *testing.T) {
+ tests := []struct {
+ name string
+ deployer Deployer
+ expectedError bool
+ }{
+ {
+ name: "successful start",
+ deployer: &MockDeployer{
+ startErr: nil,
+ },
+ expectedError: false,
+ },
+ {
+ name: "failed start",
+ deployer: &MockDeployer{
+ startErr: errors.New("start failed"),
+ },
+ expectedError: true,
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ // Save original deployer
+ originalDeployer := deploy
+ defer func() {
+ deploy = originalDeployer
+ }()
+
+ // Replace with mock deployer
+ deploy = tt.deployer
+
+ // Create a new command to test
+ testCmd := &cobra.Command{
+ Use: "test",
+ RunE: startGatewayCmd.RunE,
+ }
+
+ // Execute RunE
+ err := testCmd.RunE(testCmd, []string{})
+
+ if tt.expectedError {
+ assert.Error(t, err)
+ assert.Contains(t, err.Error(), "failed to
start gateway")
+ } else {
+ assert.NoError(t, err)
+ }
+ })
+ }
+}
+
+func TestInitDefaultValue(t *testing.T) {
+ // Reset all values to empty
+ configPath = ""
+ apiConfigPath = ""
+ logConfigPath = ""
+ logLevel = ""
+ limitCpus = ""
+ logFormat = ""
+
+ // Call initDefaultValue
+ initDefaultValue()
+
+ // Check that default values are set
+ assert.NotEmpty(t, configPath)
+ assert.NotEmpty(t, apiConfigPath)
+ assert.NotEmpty(t, logConfigPath)
+ assert.NotEmpty(t, logLevel)
+ assert.NotEmpty(t, limitCpus)
+ // logFormat can be empty as DefaultLogFormat is ""
+}
+
+func TestGatewayCmdAddedToRootCmd(t *testing.T) {
+ // Check that GatewayCmd is properly initialized
+ assert.NotNil(t, GatewayCmd)
+ assert.Equal(t, "gateway", GatewayCmd.Use)
+ assert.Equal(t, "Run dubbo go pixiu in gateway mode", GatewayCmd.Short)
+}
+
+func TestStartGatewayCmdAddedToGatewayCmd(t *testing.T) {
+ // Check that startGatewayCmd is properly initialized
+ assert.NotNil(t, startGatewayCmd)
+ assert.Equal(t, "start", startGatewayCmd.Use)
+ assert.Equal(t, "Start gateway", startGatewayCmd.Short)
+ assert.NotNil(t, startGatewayCmd.PreRunE)
+ assert.NotNil(t, startGatewayCmd.RunE)
+}
