bug: defer DeferClose before error check in multiple cmd tools #19

New Issue

Claude · 2026-02-14T16:07:06Z

Claude commented

2026-02-14 16:07:06 +00:00

Parent: #1

Description

Several command-line tools call defer extractor.DeferClose(b) or defer extractor.DeferClose(doc) before checking if the creation call returned an error:

sites/duckduckgo/cmd/duckduckgo/main.go:76-79:

b, err := browser.FromCommand(ctx, command)
defer extractor.DeferClose(b)  // deferred before error check
if err != nil {
    return fmt.Errorf("failed to create browser: %w", err)
}

sites/google/cmd/google/main.go:62-66:

b, err := browser.FromCommand(ctx, cli)
defer extractor.DeferClose(b)  // deferred before error check
if err != nil {
    return err
}

sites/wegmans/wegmans.go:63-66 and sites/aislegopher/aislegopher.go:53-56: Same pattern with b.Open().

While DeferClose nil-checks its argument, this pattern is error-prone and inconsistent with Go best practices. If a function returns both a non-nil value and an error, the defer could interact with a partially-initialized object.

Additionally, powerball and megamillions cmd tools don't call DeferClose(b) on the browser at all — the browser is leaked.

Fix

Move defer after error check:

b, err := browser.FromCommand(ctx, command)
if err != nil {
    return err
}
defer extractor.DeferClose(b)

Add defer extractor.DeferClose(b) to powerball and megamillions cmd tools.

**Parent:** #1 ## Description Several command-line tools call `defer extractor.DeferClose(b)` or `defer extractor.DeferClose(doc)` before checking if the creation call returned an error: 1. **`sites/duckduckgo/cmd/duckduckgo/main.go:76-79`**: ```go b, err := browser.FromCommand(ctx, command) defer extractor.DeferClose(b) // deferred before error check if err != nil { return fmt.Errorf("failed to create browser: %w", err) } ``` 2. **`sites/google/cmd/google/main.go:62-66`**: ```go b, err := browser.FromCommand(ctx, cli) defer extractor.DeferClose(b) // deferred before error check if err != nil { return err } ``` 3. **`sites/wegmans/wegmans.go:63-66`** and **`sites/aislegopher/aislegopher.go:53-56`**: Same pattern with `b.Open()`. While `DeferClose` nil-checks its argument, this pattern is error-prone and inconsistent with Go best practices. If a function returns both a non-nil value and an error, the defer could interact with a partially-initialized object. Additionally, **powerball and megamillions cmd tools** don't call `DeferClose(b)` on the browser at all — the browser is leaked. ## Fix Move defer after error check: ```go b, err := browser.FromCommand(ctx, command) if err != nil { return err } defer extractor.DeferClose(b) ``` Add `defer extractor.DeferClose(b)` to powerball and megamillions cmd tools.

Claude added the bug priority/high type/task labels 2026-02-14 16:07:37 +00:00

Claude referenced this issue

2026-02-14 16:09:00 +00:00

Epic: Error Handling & Correctness #1

Claude referenced this issue

2026-02-15 15:59:00 +00:00

Master plan: address all open issues (17 PRs across 5 phases) #31

Claude referenced this issue

2026-02-15 16:16:48 +00:00

bug: archive cmd uses wrong flags variable — browser flags not available #8

Claude commented

2026-02-15 16:16:49 +00:00