Skip to content

Improve error propagation where errors were silently swallowed#90

Open
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1783751218-error-handling
Open

Improve error propagation where errors were silently swallowed#90
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1783751218-error-handling

Conversation

@devin-ai-integration

Copy link
Copy Markdown

Summary

Several code paths discarded errors with _ = or err == nil guards that fell through silently, hiding real failures (corrupt writes, bad config, non-serializable responses). This tightens those spots so errors are either propagated or logged, without changing normal-path behavior.

Changes:

  • codex.writeAuthJSON — the file Close() error was dropped via defer f.Close(), so a flush-on-close failure could report success while auth.json was truncated. Now uses a named return that captures the close error:

    func writeAuthJSON(path, token string) (err error) {
        ...
        defer func() {
            if closeErr := f.Close(); closeErr != nil && err == nil {
                err = fmt.Errorf("close %s: %w", path, closeErr)
            }
        }()
        if err := json.NewEncoder(f).Encode(...); err != nil { return ... }
        return nil
    }
  • server.writeSSEpayload, _ = json.Marshal(event.Data) swallowed marshal failures and emitted a truncated/nil data: frame. Now returns fmt.Errorf("marshal SSE event %q: %w", ...), which the two callers in adapter_dispatch.go already propagate.

  • config.decodeTypedExtensionConfigdata, _ := json.Marshal; _ = json.Unmarshal(data, typed) silently returned a partially/empty-populated typed struct on failure. Now logs via slog.Warn and falls back to the raw config map instead of a silently-broken struct.

  • metrics.handleQuery / visual.ConfigForModel — both had if err == nil { _ = json.Unmarshal(data, &cfg) }, discarding the unmarshal error. Now log a warning and reset cfg = nil so the documented defaults are used deliberately rather than by accident.

  • server.writeJSON_ = json.NewEncoder(writer).Encode(payload) now logs on failure so a response that fails to serialize isn't invisible.

  • app.runHTTPServer — the handler shutdown _ = closer.Close() now logs any close error via slog.Error.

Tests

Added unit tests:

  • writeSSE propagates a marshal error (channel payload) and writes a well-formed frame for valid data.
  • writeAuthJSON writes valid JSON and returns an error when the target parent path is a regular file.

go build, go vet, gofmt -l, and go test ./... all pass.

Link to Devin session: https://app.devin.ai/sessions/3573838eee674c6fb8f34a295153326a
Requested by: @ZhiYi-R

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@ZhiYi-R ZhiYi-R self-assigned this Jul 11, 2026
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

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.

2 participants