Skip to content

Conversation

@quinchs
Copy link
Contributor

@quinchs quinchs commented Jun 28, 2023

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.

@quinchs quinchs requested a review from nsidnev June 28, 2023 18:59
@quinchs quinchs merged commit 3843342 into master Jun 28, 2023
@quinchs quinchs deleted the fix/connection-error-propigation branch June 28, 2023 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants