From 767bd86b5419ac5451dc12596050643f264e88d6 Mon Sep 17 00:00:00 2001 From: Andy Pliszka Date: Thu, 19 Sep 2024 16:50:47 -0400 Subject: [PATCH] feat: improves error reporting --- cmd/flags.go | 6 ++-- cmd/start.go | 17 ++++++---- cmd/start_test.go | 83 ++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 82 insertions(+), 24 deletions(-) diff --git a/cmd/flags.go b/cmd/flags.go index 500b2287f..aa78be2eb 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -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 { @@ -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 { diff --git a/cmd/start.go b/cmd/start.go index 8fe471d73..a289adbd6 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -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( @@ -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( diff --git a/cmd/start_test.go b/cmd/start_test.go index f3520efaf..90c4a97dc 100644 --- a/cmd/start_test.go +++ b/cmd/start_test.go @@ -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."}, }, } @@ -55,6 +65,7 @@ func TestMetricsServerFlag(t *testing.T) { } else { requireDisabledMetricsServer(t, logs, tt.wantedPort) } + requireMessages(t, logs, tt.wantedMessages) }) } } @@ -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"}, }, } @@ -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."}, }, } @@ -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() @@ -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) { @@ -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) { @@ -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) { @@ -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) {