Resource leak in batches.GetStatus() - missing defer Close()

[dimitrystd]

Problem

The GetStatus() method in api/batches/batch.go has a resource leak. It receives an io.ReadCloser from h.client.Get() but never closes it, causing memory and file descriptor leaks.

Location

File: api/batches/batch.go
Method: GetStatus() (lines 199-221)

Current Code (Problematic)

func (h httpBatch) GetStatus(projectID, batchUID string) (GetStatusResponse, error) {
    url := jobBasePath + projectID + "/batches/" + batchUID
    var response getStatusResponse
    rawMessage, code, err := h.client.Get(url, nil)  // ← Returns io.ReadCloser
    if err != nil {
        return GetStatusResponse{}, err  // ❌ Returns without closing
    }
    if code != 200 {
        return GetStatusResponse{}, fmt.Errorf("unexpected response code: %d with %s", code, rawMessage)  // ❌ Returns without closing
    }
    body, err := io.ReadAll(rawMessage)
    if err != nil {
        return GetStatusResponse{}, smerror.APIError{...}  // ❌ Returns without closing
    }
    // ... rest of the function
    return toGetStatusResponse(response), nil  // ❌ Never closed
}

Impact

• Memory Leaks: Each call to GetStatus() that returns early (error paths) leaks the HTTP response body
• File Descriptor Exhaustion: Under load, unclosed connections accumulate and can exhaust file descriptors
• Connection Pool Issues: The Go HTTP client connection pool can get stuck waiting for these unclosed bodies
• Severity: Currently low impact, but high risk if this method is called frequently or in loops

The Fix

Add a defer statement immediately after checking the error from Get():

func (h httpBatch) GetStatus(projectID, batchUID string) (GetStatusResponse, error) {
    url := jobBasePath + projectID + "/batches/" + batchUID
    var response getStatusResponse
    rawMessage, code, err := h.client.Get(url, nil)
    if err != nil {
        return GetStatusResponse{}, err
    }
    // ✅ Add this defer block
    defer func() {
        if err := rawMessage.Close(); err != nil {
            h.client.Logger.Debugf("failed to close response body: %v", err)
        }
    }()
    
    if code != 200 {
        return GetStatusResponse{}, fmt.Errorf("unexpected response code: %d with %s", code, rawMessage)
    }
    body, err := io.ReadAll(rawMessage)
    if err != nil {
        return GetStatusResponse{}, smerror.APIError{
            Cause:   err,
            URL:     url,
            Payload: []byte(fmt.Sprintf("%v", rawMessage)),
        }
    }
    if err := json.Unmarshal(body, &response); err != nil {
        return GetStatusResponse{}, fmt.Errorf("failed to unmarshal response: %w", err)
    }
    return toGetStatusResponse(response), nil
}

Reference Implementation

This pattern is already correctly implemented in api/job/job.go:

• Get() method: lines 44-48
• GetAllByName() method: lines 74-78
• Progress() method: lines 104-108

Acceptance Criteria

• Add defer close block after h.client.Get() call
• Ensure defer includes error logging for close failures
• Verify all error paths now properly close the response body
• Test under load to confirm no file descriptor leaks
• Check if other methods in batch.go have similar issues

Priority

Medium-High - Should be fixed soon to prevent production issues, but not blocking current work.

Discovery Context

Found during code review of PR #24 (CON-1896) while comparing resource management patterns between the job and batches packages.