Skip to content

bug: HandleRawResponse defers Body.Close() after reading the body #52

Description

@electather

Description

In cmd/apiutil/response.go, the HandleRawResponse function places defer r.Body.Close() after the io.ReadAll(r.Body) call. This means:

  1. If ReadAll panics, the body is never closed (resource leak).
  2. The defer is semantically misleading — it suggests the body will be closed on function exit, but it's registered too late to protect against early failures.

Steps to reproduce

  1. Review cmd/apiutil/response.go, lines ~45-55.
  2. Observe the ordering:
body, err := io.ReadAll(r.Body)  // read first
if err != nil {
    return fmt.Errorf("failed to read response body: %w", err)
}
defer r.Body.Close()  // defer registered AFTER read

Expected behaviour

defer r.Body.Close() should be placed before io.ReadAll(r.Body) to guarantee cleanup regardless of what happens during the read.

Actual behaviour

The body close is deferred after the read. If ReadAll returns an error, the function returns early and the defer is never registered, so r.Body is never closed.

seerr-cli version

All versions (code review finding).

Operating system

All platforms.

Additional context

The fix is a one-line reorder:

// Correct ordering:
defer r.Body.Close()
body, err := io.ReadAll(r.Body)
if err != nil {
    return fmt.Errorf("failed to read response body: %w", err)
}

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions