Skip to content

config: document SetStrict tradeoff and add parse error position logging#4946

Merged
lpcox merged 2 commits intomainfrom
copilot/go-fan-review-burntsushi-toml
May 1, 2026
Merged

config: document SetStrict tradeoff and add parse error position logging#4946
lpcox merged 2 commits intomainfrom
copilot/go-fan-review-burntsushi-toml

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 1, 2026

Go Fan module review for BurntSushi/toml identified two improvement opportunities in LoadFromFile: the non-obvious reason for not using Decoder.SetStrict(true) was undocumented, and parse error line/column info was not surfaced at debug level.

Changes

  • Document SetStrict(true) design decision — expands the comment before the Undecoded() loop to explain why SetStrict(true) cannot replace the manual approach: the guard_policies and guards.*.config sections are map[string]interface{} fields whose arbitrary nested keys would be incorrectly rejected as unknown by strict mode.

  • Debug-log parse error position — uses errors.As to extract toml.ParseError and emit line/column at debug level, making TOML syntax errors easier to locate when DEBUG=config:* is active. The %w wrapping for structured caller access is unchanged.

var perr toml.ParseError
if errors.As(err, &perr) {
    logConfig.Printf("TOML parse error at line %d, column %d", perr.Position.Line, perr.Position.Col)
}
return nil, fmt.Errorf("failed to parse TOML: %w", err)

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • example.com
    • Triggering command: /tmp/go-build1929706461/b513/launcher.test /tmp/go-build1929706461/b513/launcher.test -test.testlogfile=/tmp/go-build1929706461/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build1929706461/b463/vet.cfg _.a -I x_amd64/vet --gdwarf-5 /known/durationp-atomic -o w7vn10ecHFwD -W .cfg 212348/b314/ x_amd64/asm . --gdwarf2 --64 x_amd64/asm (dns block)
  • invalid-host-that-does-not-exist-12345.com
    • Triggering command: /tmp/go-build1929706461/b495/config.test /tmp/go-build1929706461/b495/config.test -test.testlogfile=/tmp/go-build1929706461/b495/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build1929706461/b368/vet.cfg 1.10.2/active_hego1.25.9 1.10.2/args.go x_amd64/vet --gdwarf-5 ions =0 x_amd64/vet swit�� _.a 212348/b151/ x_amd64/vet --gdwarf-5 transport p=/opt/hostedtoo-bool x_amd64/vet (dns block)
  • nonexistent.local
    • Triggering command: /tmp/go-build1929706461/b513/launcher.test /tmp/go-build1929706461/b513/launcher.test -test.testlogfile=/tmp/go-build1929706461/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build1929706461/b463/vet.cfg _.a -I x_amd64/vet --gdwarf-5 /known/durationp-atomic -o w7vn10ecHFwD -W .cfg 212348/b314/ x_amd64/asm . --gdwarf2 --64 x_amd64/asm (dns block)
  • slow.example.com
    • Triggering command: /tmp/go-build1929706461/b513/launcher.test /tmp/go-build1929706461/b513/launcher.test -test.testlogfile=/tmp/go-build1929706461/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build1929706461/b463/vet.cfg _.a -I x_amd64/vet --gdwarf-5 /known/durationp-atomic -o w7vn10ecHFwD -W .cfg 212348/b314/ x_amd64/asm . --gdwarf2 --64 x_amd64/asm (dns block)
  • this-host-does-not-exist-12345.com
    • Triggering command: /tmp/go-build1929706461/b522/mcp.test /tmp/go-build1929706461/b522/mcp.test -test.testlogfile=/tmp/go-build1929706461/b522/testlog.txt -test.paniconexit0 -test.timeout=10m0s 2123�� .cfg -trimpath x_amd64/vet -p mime/quotedprint/usr/bin/runc -lang=go1.25 x_amd64/vet .cfg�� 212348/b306/_pkg_.a -trimpath x_amd64/vet -p .io/otel/exportedocker-cli-plugin-metadata -lang=go1.24 x_amd64/vet (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI linked an issue May 1, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Review Go module BurntSushi/toml for config loading config: document SetStrict tradeoff and add parse error position logging May 1, 2026
Copilot finished work on behalf of lpcox May 1, 2026 15:04
Copilot AI requested a review from lpcox May 1, 2026 15:04
@lpcox lpcox marked this pull request as ready for review May 1, 2026 15:05
Copilot AI review requested due to automatic review settings May 1, 2026 15:05
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

Improves TOML config loading ergonomics in internal/config by clarifying why strict TOML decoding can’t be enabled globally and by adding debug-only logging that pinpoints TOML syntax error locations.

Changes:

  • Add debug logging of TOML parse error line/column by extracting toml.ParseError via errors.As.
  • Expand the unknown-field checking comment to document why Decoder.SetStrict(true) isn’t viable due to dynamic map[string]interface{} sections.
Show a summary per file
File Description
internal/config/config_core.go Logs TOML parse error position at debug level and documents the strict-decoder tradeoff around dynamic config sections.

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: 0

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.

[go-fan] Go Module Review: BurntSushi/toml

3 participants