enbility / ship-go / 28788123488

93%

Build Type

push

github

Committed by web-flow

Commit Message

ship: tear down connection on a failed spine data write (#86)

Fix https://github.com/evcc-io/evcc/issues/31465

Found while chasing an intermittent double-connection related test hang
in evcc (evcc-io/evcc#31493) that reproduces on top of #85's dedup fix.

`sendSpineData` only calls `CloseConnection` when
`IsDataConnectionClosed()` already reports the connection closed:

```go
if isClosed, err := c.dataWriter.IsDataConnectionClosed(); isClosed {
    c.CloseConnection(false, 0, "")
    return err
}
...
err = c.dataWriter.WriteMessageToWebsocketConnection(shipMsg)
if err != nil {
    logging.Log().Debug("error sending message: ", err)
    return err
}
```

If the actual websocket write fails for any other reason (e.g. the
remote already closed the connection before this side's writer noticed),
the error is logged and swallowed, but the connection is never torn
down. The `ShipConnection` stays registered as the current writer for
that SKI, so every subsequent send silently fails forever and the SKI is
never freed for a fresh connection attempt.

This surfaced concretely while stress-testing a double-connection race
scenario: after one side's connection was implicitly invalidated, the
other kept retrying sends against it every couple of seconds
indefinitely ("Error sending spine message: connection closed for remote
SKI ...", repeating), and no reconnect was ever attempted.

Fix: also call `CloseConnection(false, 0, "")` when the write itself
errors, mirroring the existing `IsDataConnectionClosed` branch. Updated
`TestSendSpineData_WriteError` to expect the resulting
`HandleConnectionClosed` call.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Coverage Stats

3 of 3 new or added lines in 1 file covered. (100.0%)

2 existing lines in 1 file now uncovered.

5806 of 6244 relevant lines covered (92.99%)

12481.14 hits per line