-
Notifications
You must be signed in to change notification settings - Fork 10
Standardize API error logs #70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,12 @@ | ||
| package main | ||
|
|
||
| import ( | ||
| "log" | ||
|
|
||
| "github.com/julsemaan/anyfile-notepad/api/internal/app" | ||
| "github.com/julsemaan/anyfile-notepad/api/internal/logging" | ||
| ) | ||
|
|
||
| func main() { | ||
| if err := app.Run(app.LoadConfigFromEnv()); err != nil { | ||
| log.Fatal(err) | ||
| logging.Fatalf("%v", err) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| package logging | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "log" | ||
| "os" | ||
| "strings" | ||
| ) | ||
|
|
||
| const errorPrefix = "ERROR:" | ||
|
|
||
| func output(message string) { | ||
| _ = log.Default().Output(3, message) | ||
| } | ||
|
|
||
| func Error(args ...interface{}) { | ||
| msg := strings.TrimSuffix(fmt.Sprintln(args...), "\n") | ||
| if msg == "" { | ||
| output(errorPrefix) | ||
| return | ||
| } | ||
|
|
||
| output(errorPrefix + " " + msg) | ||
| } | ||
|
|
||
| func Errorf(format string, args ...interface{}) { | ||
| output(errorPrefix + " " + fmt.Sprintf(format, args...)) | ||
| } | ||
|
|
||
| func Fatalf(format string, args ...interface{}) { | ||
| output(errorPrefix + " " + fmt.Sprintf(format, args...)) | ||
| os.Exit(1) | ||
| } | ||
|
Comment on lines
+16
to
+33
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| package logging | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "errors" | ||
| "log" | ||
| "os" | ||
| "os/exec" | ||
| "strings" | ||
| "testing" | ||
| ) | ||
|
|
||
| func captureLogOutput(t *testing.T) *bytes.Buffer { | ||
| t.Helper() | ||
|
|
||
| buf := &bytes.Buffer{} | ||
| originalWriter := log.Writer() | ||
| originalFlags := log.Flags() | ||
| originalPrefix := log.Prefix() | ||
|
|
||
| log.SetOutput(buf) | ||
| log.SetFlags(0) | ||
| log.SetPrefix("") | ||
|
|
||
| t.Cleanup(func() { | ||
| log.SetOutput(originalWriter) | ||
| log.SetFlags(originalFlags) | ||
| log.SetPrefix(originalPrefix) | ||
| }) | ||
|
|
||
| return buf | ||
| } | ||
|
|
||
| func TestErrorAddsPrefix(t *testing.T) { | ||
| buf := captureLogOutput(t) | ||
|
|
||
| Error("unable to send email", 42) | ||
|
|
||
| if got := buf.String(); !strings.Contains(got, "ERROR: unable to send email 42") { | ||
| t.Fatalf("expected ERROR prefix in output, got %q", got) | ||
| } | ||
| } | ||
|
|
||
| func TestErrorfAddsPrefix(t *testing.T) { | ||
| buf := captureLogOutput(t) | ||
|
|
||
| Errorf("unable to send email %d", 42) | ||
|
|
||
| if got := buf.String(); !strings.Contains(got, "ERROR: unable to send email 42") { | ||
| t.Fatalf("expected ERROR prefix in output, got %q", got) | ||
| } | ||
| } | ||
|
|
||
| func TestFatalfAddsPrefixAndExits(t *testing.T) { | ||
| if os.Getenv("LOGGING_FATALF_TEST") == "1" { | ||
| log.SetOutput(os.Stderr) | ||
| log.SetFlags(0) | ||
| log.SetPrefix("") | ||
| Fatalf("unable to send email %d", 42) | ||
| return | ||
| } | ||
|
Comment on lines
+55
to
+61
|
||
|
|
||
| cmd := exec.Command(os.Args[0], "-test.run=TestFatalfAddsPrefixAndExits") | ||
| cmd.Env = append(os.Environ(), "LOGGING_FATALF_TEST=1") | ||
|
|
||
| var stderr bytes.Buffer | ||
| cmd.Stderr = &stderr | ||
|
|
||
| err := cmd.Run() | ||
| var exitErr *exec.ExitError | ||
| if !errors.As(err, &exitErr) { | ||
| t.Fatalf("expected process exit error, got %v", err) | ||
| } | ||
| if exitErr.ExitCode() != 1 { | ||
| t.Fatalf("expected exit code 1, got %d", exitErr.ExitCode()) | ||
| } | ||
|
|
||
| if got := stderr.String(); !strings.Contains(got, "ERROR: unable to send email 42") { | ||
| t.Fatalf("expected ERROR prefix in fatal output, got %q", got) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This error is logged but the application continues to run without StatsD (statsConn can be nil). The updated message drops the previous “warning”/non-fatal signal, which can be misleading operationally; consider including explicit wording like “continuing without metrics” (while still keeping the required "ERROR:" prefix).