Skip to content

Commit

Permalink
feat: improves error reporting
Browse files Browse the repository at this point in the history
  • Loading branch information
AntiTyping committed Sep 19, 2024
1 parent 6cb53dc commit 767bd86
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 24 deletions.
6 changes: 4 additions & 2 deletions cmd/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,8 @@ func debugServerFlags(v *viper.Viper, cmd *cobra.Command) *cobra.Command {
flagDebugListenAddr,
"",
"address to use for debug and metrics server. By default, "+
"will be the debug-listen-addr parameter in the global config.",
"will be the debug-listen-addr parameter in the global config. "+
"Make sure to enable debug server using --enable-debug-server flag.",
)

if err := v.BindPFlag(flagDebugListenAddr, cmd.Flags().Lookup(flagDebugListenAddr)); err != nil {
Expand All @@ -450,7 +451,8 @@ func metricsServerFlags(v *viper.Viper, cmd *cobra.Command) *cobra.Command {
flagMetricsListenAddr,
"",
"address to use for metrics server. By default, "+
"will be the metrics-listen-addr parameter in the global config.",
"will be the metrics-listen-addr parameter in the global config. "+
"Make sure to enable metrics server using --enable-metrics-server flag.",
)

if err := v.BindPFlag(flagMetricsListenAddr, cmd.Flags().Lookup(flagMetricsListenAddr)); err != nil {
Expand Down
17 changes: 11 additions & 6 deletions cmd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,12 @@ func setupMetricsServer(cmd *cobra.Command, a *appState, err error, chains map[s
return nil, err
}

if flagEnableMetricsServer == false || metricsListenAddr == "" {
a.log.Warn("Disabled metrics server due to missing metrics-listen-addr setting in config file.")
if flagEnableMetricsServer == false {
a.log.Info("Metrics server is disabled. You can enable it using --enable-metrics-server flag.")
} else if metricsListenAddr == "" {
a.log.Warn("Disabled metrics server due to missing metrics-listen-addr setting in config file or --metrics-listen-addr flag.")
} else {
a.log.Info("Metrics server is enabled.")
a.log.Info("Metrics server is enabled")
ln, err := net.Listen("tcp", metricsListenAddr)
if err != nil {
a.log.Error(
Expand Down Expand Up @@ -235,10 +237,13 @@ func setupDebugServer(cmd *cobra.Command, a *appState, err error) error {
return err
}

if flagEnableDebugServer == false || debugListenAddr == "" {
a.log.Warn("Disabled debug server due to missing debug-listen-addr setting in config file.")
if flagEnableDebugServer == false {
a.log.Info("Debug server is disabled. You can enable it using --enable-debug-server flag.")
} else if debugListenAddr == "" {
a.log.Warn("Disabled debug server due to missing debug-listen-addr setting in config file or --debug-listen-addr flag.")
} else {
a.log.Warn("SECURITY WARNING! Debug server is enabled. It should only be used with caution and proper security.")
a.log.Info("Debug server is enabled")
a.log.Warn("SECURITY WARNING! Debug server should only be run with caution and proper security in place.")
ln, err := net.Listen("tcp", debugListenAddr)
if err != nil {
a.log.Error(
Expand Down
83 changes: 67 additions & 16 deletions cmd/start_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,24 +22,34 @@ func TestMetricsServerFlag(t *testing.T) {
t.Parallel()

tests := []struct {
args []string
wantedPort int
wantedRunning bool
args []string
wantedPort int
wantedRunning bool
wantedMessages []string
}{
{
[]string{"start"},
0,
false,
[]string{"Metrics server is disabled. You can enable it using --enable-metrics-server flag."},
},
{
[]string{"start", "--enable-metrics-server"},
relayermetrics.MetricsServerPort,
true,
[]string{"Metrics server is enabled", "Metrics server listening"},
},
{
[]string{"start", "--enable-metrics-server", "--metrics-listen-addr", "127.0.0.1:7778"},
7778,
true,
[]string{"Metrics server is enabled", "Metrics server listening"},
},
{
[]string{"start", "--metrics-listen-addr", "127.0.0.1:7778"},
0,
false,
[]string{"Metrics server is disabled. You can enable it using --enable-metrics-server flag."},
},
}

Expand All @@ -55,6 +65,7 @@ func TestMetricsServerFlag(t *testing.T) {
} else {
requireDisabledMetricsServer(t, logs, tt.wantedPort)
}
requireMessages(t, logs, tt.wantedMessages)
})
}
}
Expand All @@ -63,28 +74,32 @@ func TestMetricsServerConfig(t *testing.T) {
t.Parallel()

tests := []struct {
args []string
newSetting string
wantedPort int
serverRunning bool
args []string
newSetting string
wantedPort int
serverRunning bool
wantedMessages []string
}{
{
[]string{"start"},
"",
0,
false,
[]string{"Metrics server is disabled. You can enable it using --enable-metrics-server flag."},
},
{
[]string{"start", "--enable-metrics-server"},
"metrics-listen-addr: 127.0.0.1:6184",
6184,
true,
[]string{"Metrics server is enabled", "Metrics server listening"},
},
{
[]string{"start", "--enable-metrics-server", "--metrics-listen-addr", "127.0.0.1:7184"},
"",
7184,
true,
[]string{"Metrics server is enabled", "Metrics server listening"},
},
}

Expand All @@ -103,32 +118,50 @@ func TestMetricsServerConfig(t *testing.T) {
} else {
requireDisabledMetricsServer(t, logs, tt.wantedPort)
}
requireMessages(t, logs, tt.wantedMessages)
})
}
}

func TestMissingMetricsListenAddr(t *testing.T) {
sys := setupRelayer(t)

logs, logger := setupLogger()

updateConfig(t, sys, "metrics-listen-addr: 127.0.0.1:5184", "")

sys.MustRunWithLogger(t, logger, []string{"start", "--enable-metrics-server"}...)

requireDisabledMetricsServer(t, logs, 0)
requireMessage(t, logs, "Disabled metrics server due to missing metrics-listen-addr setting in config file or --metrics-listen-addr flag.")
}

func TestDebugServerFlag(t *testing.T) {
t.Parallel()

tests := []struct {
args []string
wantedPort int
wantedRunning bool
args []string
wantedPort int
wantedRunning bool
wantedMessages []string
}{
{
[]string{"start"},
0,
false,
[]string{"Debug server is disabled. You can enable it using --enable-debug-server flag."},
},
{
[]string{"start", "--enable-debug-server"},
relaydebug.DebugServerPort,
true,
[]string{"Debug server is enabled", "SECURITY WARNING! Debug server should only be run with caution and proper security in place."},
},
{
[]string{"start", "--enable-debug-server", "--debug-listen-addr", "127.0.0.1:7777"},
7777,
true,
[]string{"Debug server is enabled", "SECURITY WARNING! Debug server should only be run with caution and proper security in place."},
},
}

Expand All @@ -144,10 +177,24 @@ func TestDebugServerFlag(t *testing.T) {
} else {
requireDisabledDebugServer(t, logs, tt.wantedPort)
}
requireMessages(t, logs, tt.wantedMessages)
})
}
}

func TestMissingDebugListenAddr(t *testing.T) {
sys := setupRelayer(t)

logs, logger := setupLogger()

updateConfig(t, sys, "debug-listen-addr: 127.0.0.1:5183", "")

sys.MustRunWithLogger(t, logger, []string{"start", "--enable-debug-server"}...)

requireDisabledMetricsServer(t, logs, 0)
requireMessage(t, logs, "Disabled debug server due to missing debug-listen-addr setting in config file or --debug-listen-addr flag.")
}

func TestDebugServerConfig(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -196,7 +243,6 @@ func requireDisabledMetricsServer(t *testing.T, logs *observer.ObservedLogs, por
defer conn.Close()
}
require.Error(t, err, "Server should be disabled")
require.Len(t, logs.FilterMessage("Disabled debug server due to missing debug-listen-addr setting in config file.").All(), 1)
}

func requireRunningMetricsServer(t *testing.T, logs *observer.ObservedLogs, port int) {
Expand All @@ -208,8 +254,6 @@ func requireRunningMetricsServer(t *testing.T, logs *observer.ObservedLogs, port
res, err := http.Get(fmt.Sprintf("http://127.0.0.1:%d/metrics", port))
require.NoError(t, err)
require.Equal(t, res.StatusCode, 200)
require.Len(t, logs.FilterMessage("Metrics server listening").All(), 1)
require.Len(t, logs.FilterMessage("Metrics server is enabled.").All(), 1)
}

func requireDisabledDebugServer(t *testing.T, logs *observer.ObservedLogs, port int) {
Expand All @@ -218,7 +262,6 @@ func requireDisabledDebugServer(t *testing.T, logs *observer.ObservedLogs, port
defer conn.Close()
}
require.Error(t, err, "Server should be disabled")
require.Len(t, logs.FilterMessage("Disabled debug server due to missing debug-listen-addr setting in config file.").All(), 1)
}

func requireRunningDebugServer(t *testing.T, logs *observer.ObservedLogs, port int) {
Expand All @@ -230,8 +273,16 @@ func requireRunningDebugServer(t *testing.T, logs *observer.ObservedLogs, port i
res, err := http.Get(fmt.Sprintf("http://127.0.0.1:%d/debug/pprof/goroutine", port))
require.NoError(t, err)
require.Equal(t, res.StatusCode, 200)
require.Len(t, logs.FilterMessage("Debug server listening").All(), 1)
require.Len(t, logs.FilterMessage("SECURITY WARNING! Debug server is enabled. It should only be used with caution and proper security.").All(), 1)
}

func requireMessages(t *testing.T, logs *observer.ObservedLogs, messages []string) {
for _, message := range messages {
requireMessage(t, logs, message)
}
}

func requireMessage(t *testing.T, logs *observer.ObservedLogs, message string) {
require.Len(t, logs.FilterMessage(message).All(), 1, fmt.Sprintf("Expected message '%s' not found in logs", message), logs.All())
}

func setupLogger() (*observer.ObservedLogs, *zap.Logger) {
Expand Down

0 comments on commit 767bd86

Please sign in to comment.