fix: properly handle io.Reader EOF in Push function

[johnhorsema]

Fix issue where Node.js nodes couldn't read body in subsequent requests after POST requests to Go gateway.

The original code incorrectly required both io.EOF and nn==0 to break the read loop, but io.Reader can return EOF with data on final read. This caused premature stream closure and left connections in inconsistent state.

Changes:

• Process data (nn > 0) before checking for errors
• Send EOF signal (empty DataPacket) only after all data is sent
• Fix variable shadowing by using errSend instead of err

[johnhorsema]

@greptileai review this

[greptile-apps]

Greptile Summary

This PR fixes the Push function in provider.go to correctly handle the case where io.Reader.Read returns both data (nn > 0) and io.EOF in a single call, which is permitted by the io.Reader contract. The original code's err == io.EOF && nn == 0 break condition could cause an unnecessary second Read call after EOF and, depending on the ReadCloser implementation, leave connections in an inconsistent state.

• The loop now processes any returned data (nn > 0) before inspecting the error, and sends the EOF signal (empty DataPacket) immediately when io.EOF is detected rather than waiting for a subsequent zero-byte read.
• Variable shadowing in the send-error path is fixed by using errSend instead of re-declaring err.
• An inline comment documents the intentional decision to forward partial chunks to the peer before propagating non-EOF read errors.

Confidence Score: 5/5

Safe to merge — the change is a focused, well-reasoned correction to a single read loop that now faithfully follows the io.Reader contract.

The rewritten loop correctly handles all four read outcomes (data-only, data+EOF, EOF-only, other error) without calling Read after EOF. The EOF signal packet is now sent exactly once and at the right moment, matching what the Node.js and Go receivers already expect. The variable-shadowing fix eliminates a subtle source of confusion with no behavioral downside. No new error paths are introduced.

No files require special attention.

Important Files Changed

Filename	Overview
provider.go	Read loop rewritten to correctly honour the io.Reader contract (process data before error, send EOF signal immediately on io.EOF); variable shadowing fixed with errSend.

Sequence Diagram

sequenceDiagram
    participant C as Push caller
    participant P as Provider.Push
    participant R as body (io.ReadCloser)
    participant N as Peer (Node.js / Go)

    C->>P: Push(services, headers, body)
    P->>N: OpcodeServiceRequest (stream header)

    loop Read body
        P->>R: body.Read(buf)
        alt "nn > 0, nil"
            R-->>P: n bytes, nil
            P->>N: OpcodeData [n bytes]
        else "nn > 0, io.EOF (final chunk with data)"
            R-->>P: n bytes, io.EOF
            P->>N: OpcodeData [n bytes]
            P->>N: "OpcodeData [] (empty = EOF signal)"
            Note over P: break
        else "nn == 0, io.EOF"
            R-->>P: 0 bytes, io.EOF
            P->>N: "OpcodeData [] (empty = EOF signal)"
            Note over P: break
        else other error
            R-->>P: n bytes, err
            P->>N: "OpcodeData [n bytes] (if nn > 0)"
            P->>N: CloseStreamWithError
            P-->>C: nil, error
        end
    end

    N-->>P: stream.wg.Done
    P-->>C: Stream, nil

Loading

_{Reviews (3): Last reviewed commit: "Update provider.go" | Re-trigger Greptile}

[greptile-apps]

Greptile Summary

This PR fixes a subtle but real bug in the Push function's body-reading loop, where the old code required both err == io.EOF and nn == 0 to break out of the loop. The Go io.Reader contract explicitly allows returning data and io.EOF in the same call, meaning the old code could loop indefinitely on readers that signal EOF with the final bytes.

• The loop is restructured to send any buffered data first (nn > 0) and then immediately send an explicit empty DataPacket EOF signal and break when err == io.EOF, matching what node.go already expects as the stream-close signal.
• Variable shadowing in the send-error path is cleaned up by renaming the inner variable to errSend, making the control flow easier to follow.

Confidence Score: 4/5

The change is safe to merge; it correctly aligns the read loop with the Go io.Reader contract and the empty-DataPacket EOF signal that the receiver already expects.

The core fix is sound — processing data before inspecting the error, then sending the explicit empty EOF packet and breaking, is the right pattern. The one subtlety is that on a non-EOF read error the new code now forwards any partial chunk it read to the peer before closing the stream, whereas the old code discarded it. This is an error-path behaviour change that the receiver currently has no special handling for, but it does not break anything in practice.

provider.go — specifically the non-EOF error path inside the read loop where partial data is now forwarded before stream closure.

Important Files Changed

Filename	Overview
provider.go	Read loop restructured to correctly handle io.Reader returning data alongside io.EOF; error path now sends partial data before closing stream (minor behavior change on non-EOF read errors).

Sequence Diagram

sequenceDiagram
    participant C as Caller
    participant P as Push (provider.go)
    participant R as body io.Reader
    participant N as Remote Node

    C->>P: Push(services, headers, body)
    P->>N: ServiceRequestPacket (OpcodeServiceRequest)

    loop Read body
        P->>R: body.Read(buf)
        alt "nn > 0, err = nil"
            R-->>P: data chunk
            P->>N: "DataPacket{Data: buf[:nn]}"
        else "nn > 0, err = io.EOF"
            R-->>P: final chunk + EOF
            P->>N: "DataPacket{Data: buf[:nn]}"
            P->>N: "DataPacket{Data: []} — EOF signal"
            Note over P,N: Break loop
        else "nn = 0, err = io.EOF"
            R-->>P: clean EOF
            P->>N: "DataPacket{Data: []} — EOF signal"
            Note over P,N: Break loop
        else non-EOF error
            R-->>P: err
            P->>N: CloseStreamWithError
            P-->>C: return nil, err
        end
    end

    N-->>P: ServiceResponsePacket
    P-->>C: return stream, nil

Loading

_{Reviews (2): Last reviewed commit: "fix: properly handle io.Reader EOF in Pu..." | Re-trigger Greptile}