Skip to content

Commit

Permalink
Remove assertion from logging observers onConnectionSelected
Browse files Browse the repository at this point in the history
Motivation:

If request is retried multiple times, `onConnectionSelected` callback
will be invoked multiple times (for every retry attempt) when logging
observers are applied at the client level.

Modifications:

- Remove `assert this.connInfo == null` from
`LoggingGrpcLifecycleObserver` and `LoggingHttpLifecycleObserver`;
- Clarify expectations in `HttpLifecycleObserver` javadoc;

Result:

No `AssertionError` in tests that can retry.
  • Loading branch information
idelpivnitskiy committed Jul 5, 2023
1 parent 3843dd4 commit 3d0921b
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ private LoggingGrpcExchangeObserver(final FixedLevelLogger logger) {

@Override
public void onConnectionSelected(final ConnectionInfo info) {
assert this.connInfo == null;
this.connInfo = info;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ public interface HttpLifecycleObserver {

/**
* Callback when a new HTTP exchange starts.
* <p>
* Depending on the order in which the observer is applied, this callback can be invoked either for every retry
* attempt (if an observer is added after retrying filter) or only once per exchange.
*
* @return an {@link HttpExchangeObserver} that provides visibility into exchange events
*/
Expand All @@ -56,6 +59,8 @@ interface HttpExchangeObserver {

/**
* Callback when a connection is selected for this exchange execution.
* <p>
* This callback may be invoked for every retry attempt.
*
* @param info {@link ConnectionInfo} of the selected connection
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ private LoggingHttpExchangeObserver(final FixedLevelLogger logger) {

@Override
public void onConnectionSelected(final ConnectionInfo info) {
assert this.connInfo == null;
this.connInfo = info;
}

Expand Down

0 comments on commit 3d0921b

Please sign in to comment.