edgedb / edgedb-java

The official Java client library for EdgeDB
Apache License 2.0
19 stars 4 forks source link

Fix retry connection on send #12

Closed quinchs closed 1 year ago

quinchs commented 1 year ago

Summary

The binding would attempt to reconnect in the send method if the connection wasn't established: https://github.com/edgedb/edgedb-java/blob/46c513285faffee9992e47447eb542f5c7a158ae/src/driver/src/main/java/com/edgedb/driver/binary/duplexers/ChannelDuplexer.java#L200-L213 The issue with this is that the connect method on the EdgeDBBinaryClient would be recursively called if the underlying connection was closed during the send function:

EdgeDBBinaryClient#connect()
├─ EdgeDBBinaryClient#connectInternal()
│  └─ EdgeDBBinaryClient#retryableConnect()
│     └─ ChannelDuplexer#send()
│        ├─ IsConnected?
└────────╬━┸- Channel#write()
         └─ EdgeDBBinaryClient#reconnect()
            └─ EdgeDBBinaryClient#connect()
              └─ duplicate frames hidden...

The way the attempts were calculated here are locals to only the method, they're not passed down the call hierarchy so this bug would occur.

Changes

The send method will now recall itself only when an EdgeDBException is thrown with the ShouldRetry flag. Any other exceptions are propagated to the caller which is responsible for handling them.