Skip to content

Don't Panic #221

@spikecurtis

Description

@spikecurtis

We had an issue that was ultimately caused by a bad slog call that panicked. Panicking on a log call is pretty severe and unfriendly---and generally should be reserved for cases where the system is left in an inconsistent or dangerous state.

The panicking call was like

logger.Error(ctx, "something bad happened: %v", err)

This is not the correct calling format for slog, but is very similar to return fmt.Errorf and how other logging libraries work. With the rise of LLM-based code generation, this kind of thing is increasingly easy to get wrong.

Slog's logging calls take ...any in the arguments, so there is nothing the Go compiler can do to ensure you're calling it right.

One thing we could consider is making the calls take ...Field. This would allow the compiler to catch errors like the one above, but would be breaking change and we'd loose support for calling like

logger.Info(ctx, "dug a hole", "depth", depth, "diameter", diameter)

This form is quite rare in github.com/coder/coder so would be a cheap migration to clean up.

Alternatively, we could modify the logging so that it doesn't panic, and instead emits a CRITICAL log noting the bad logging call instead. This keeps backward compatibility, but at the cost of much less visibility --- problems can only be caught at runtime, and only if you are paying attention to the logs.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions