Skip to content

fix(envutil): route GetIntFromEnv warnings through logger when provided#29195

Merged
pelikhan merged 2 commits intomainfrom
copilot/fix-getintfromenv-logging
Apr 29, 2026
Merged

fix(envutil): route GetIntFromEnv warnings through logger when provided#29195
pelikhan merged 2 commits intomainfrom
copilot/fix-getintfromenv-logging

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 29, 2026

GetIntFromEnv accepted a *logger.Logger but only used it for success messages — warnings for invalid/out-of-bounds values always went directly to os.Stderr, making the logger parameter misleading and preventing callers from controlling warning output.

Changes

  • pkg/envutil/envutil.go: Introduced a local warn closure that routes through log.Printf when a logger is provided, falling back to fmt.Fprintln(os.Stderr, ...) only when log is nil
warn := func(msg string) {
    if log != nil {
        log.Printf("WARNING: %s", msg)
    } else {
        fmt.Fprintln(os.Stderr, console.FormatWarningMessage(msg))
    }
}

No behavior change for callers passing nil as the logger.

Copilot AI changed the title [WIP] Fix logging bypass in GetIntFromEnv method fix(envutil): route GetIntFromEnv warnings through logger when provided Apr 29, 2026
Copilot AI requested a review from pelikhan April 29, 2026 21:36
@pelikhan pelikhan marked this pull request as ready for review April 29, 2026 21:51
Copilot AI review requested due to automatic review settings April 29, 2026 21:51
@pelikhan pelikhan merged commit 7207311 into main Apr 29, 2026
16 checks passed
@pelikhan pelikhan deleted the copilot/fix-getintfromenv-logging branch April 29, 2026 21:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates GetIntFromEnv to route warning output through the provided *logger.Logger (instead of always writing directly to os.Stderr), aiming to make the log parameter reflect actual warning behavior.

Changes:

  • Added a local warn helper in GetIntFromEnv to centralize warning emission and choose between logger vs. direct stderr output.
  • Updated parse/out-of-bounds branches to call the shared warn helper.
  • Updated the function comment to reflect logger-based warning routing.
Show a summary per file
File Description
pkg/envutil/envutil.go Centralizes warning emission and routes warnings via logger when provided.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 1/1 changed files
  • Comments generated: 1

Comment thread pkg/envutil/envutil.go
Comment on lines +29 to +36
// Invalid values trigger warning messages to stderr, or through the logger if provided.
func GetIntFromEnv(envVar string, defaultValue, minValue, maxValue int, log *logger.Logger) int {
warn := func(msg string) {
if log != nil {
log.Printf("WARNING: %s", msg)
} else {
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(msg))
}
@github-actions github-actions Bot mentioned this pull request Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pkg/envutil: GetIntFromEnv bypasses structured logger for warning messages

3 participants