Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions rollbar-okhttp/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
# Rollbar OkHttp Integration

This module provides an [OkHttp Interceptor](https://square.github.io/okhttp/features/interceptors/) that automatically captures network telemetry for the Rollbar Java SDK.

It records:

- **Network telemetry events** for HTTP responses with status code `>= 400` (client and server errors).
- **Error events** for connection failures, timeouts, and other I/O exceptions.

## Installation

### Gradle (Kotlin DSL)

```kotlin
dependencies {
implementation("com.rollbar:rollbar-okhttp:<version>")
implementation("com.squareup.okhttp3:okhttp:<okhttp-version>")
}
```

### Gradle (Groovy)

```groovy
dependencies {
implementation 'com.rollbar:rollbar-okhttp:<version>'
implementation 'com.squareup.okhttp3:okhttp:<okhttp-version>'
}
```

## Usage

### 1. Implement `NetworkTelemetryRecorder`

```java
NetworkTelemetryRecorder recorder = new NetworkTelemetryRecorder() {
@Override
public void recordNetworkEvent(Level level, String method, String url, String statusCode) {
rollbar.recordNetworkEventFor(level, method, url, statusCode);
}

@Override
public void recordErrorEvent(Exception exception) {
rollbar.log(exception);
}
};
```

### 2. Add the interceptor to your OkHttpClient

```java
OkHttpClient client = new OkHttpClient.Builder()
.addInterceptor(new RollbarOkHttpInterceptor(recorder))
.build();
```

### 3. Make requests as usual

```java
Request request = new Request.Builder()
.url("https://api.example.com/data")
.build();

Response response = client.newCall(request).execute();
```

The interceptor will automatically record telemetry events to Rollbar without interfering with the request/response flow.

## Behavior

| Scenario | Action |
|-----------------------------------|---------------------------------------------------------|
| Recorder is `null` | No telemetry or log is recorded |
| Response status `< 400` | No telemetry recorded, response returned normally |
| Response status `>= 400` | Records a network telemetry event with `Level.CRITICAL` |
| Connection failure / timeout | Records an error event, then rethrows the `IOException` |
24 changes: 24 additions & 0 deletions rollbar-okhttp/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
plugins {
id("java")
}

group = "com.rollbar.okhttp"
version = "2.2.0"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 rollbar-okhttp/build.gradle.kts hardcodes version = "2.2.0" instead of relying on the root project's version management, so future release bumps to VERSION_NAME will not update this module's published version. Remove the local version declaration to let the root allprojects { version = versionName } take effect, keeping rollbar-okhttp in sync with the rest of the SDK.

Extended reasoning...

What the bug is and how it manifests

The new rollbar-okhttp module sets version = "2.2.0" directly in its build.gradle.kts (line 6). In Gradle, each subproject's build script is evaluated after the root project's allprojects block, which means a local version = ... assignment in a subproject overrides whatever the root block set. As a result, rollbar-okhttp will always advertise itself as version 2.2.0 in its published Maven artifact regardless of what VERSION_NAME is set to.

The specific code path that triggers it

The root build.gradle.kts reads val versionName = rootProject.extra["VERSION_NAME"] as String from gradle.properties and then applies it inside allprojects { version = versionName }. rollbar-okhttp's local version = "2.2.0" in its own build script then replaces that value during subproject configuration. All other submodules (rollbar-java, rollbar-api, rollbar-web, rollbar-reactive-streams, etc.) omit a local version declaration and therefore correctly inherit the root-level value.

Why existing code doesn't prevent it

There is no enforcement mechanism (e.g., a build check or version-consistency task) that asserts all published modules share the same version. The hardcoded string happens to equal the current VERSION_NAME (also "2.2.0"), so the problem is invisible today.

What the impact would be

When the project next bumps VERSION_NAME (say to 2.3.0), every other module will publish as 2.3.0, but rollbar-okhttp will still publish as 2.2.0. Consumers who declare a BOM or uniform version constraint for all rollbar artifacts will get a resolution conflict or silently use the wrong (stale) artifact.

How to fix it

Remove line 6 (version = "2.2.0") from rollbar-okhttp/build.gradle.kts. The root allprojects block will then set the version correctly for this module just as it does for every other module.

Step-by-step proof

  1. gradle.properties sets VERSION_NAME=2.2.0.
  2. Root build.gradle.kts evaluates allprojects { version = "2.2.0" } → rollbar-okhttp.version = "2.2.0" ✓
  3. Gradle then evaluates rollbar-okhttp/build.gradle.kts; the line version = "2.2.0" reassigns the same value (no visible effect right now).
  4. Developer bumps VERSION_NAME=2.3.0 in gradle.properties.
  5. Root allprojects sets rollbar-okhttp.version = "2.3.0" ✓ — but is immediately overwritten in step 6.
  6. rollbar-okhttp/build.gradle.kts still has version = "2.2.0"; Gradle overwrites to "2.2.0".
  7. ./gradlew :rollbar-okhttp:publish publishes com.rollbar:rollbar-okhttp:2.2.0 while all peer artifacts publish as 2.3.0.


repositories {
mavenCentral()
}

dependencies {
testImplementation(platform("org.junit:junit-bom:5.14.3"))
testImplementation("org.junit.jupiter:junit-jupiter")
testRuntimeOnly("org.junit.platform:junit-platform-launcher")
testImplementation("com.squareup.okhttp3:mockwebserver:5.3.2")
testImplementation("org.mockito:mockito-core:5.23.0")
implementation("com.squareup.okhttp3:okhttp:5.3.2")
api(project(":rollbar-api"))
}

tasks.test {
useJUnitPlatform()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.rollbar.okhttp;

import com.rollbar.api.payload.data.Level;

public interface NetworkTelemetryRecorder {
void recordNetworkEvent(Level level, String method, String url, String statusCode);

void recordErrorEvent(Exception exception);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package com.rollbar.okhttp;

import com.rollbar.api.payload.data.Level;

import java.io.IOException;

import okhttp3.Interceptor;
import okhttp3.Request;
import okhttp3.Response;

public class RollbarOkHttpInterceptor implements Interceptor {

private final NetworkTelemetryRecorder recorder;

public RollbarOkHttpInterceptor(NetworkTelemetryRecorder recorder) {
this.recorder = recorder;
}

@Override
public Response intercept(Chain chain) throws IOException {
Request request = chain.request();

try {
Response response = chain.proceed(request);

if (response.code() >= 400 && recorder != null) {
recorder.recordNetworkEvent(
Level.CRITICAL,
request.method(),
request.url().toString(),
String.valueOf(response.code()));
}

return response;

} catch (IOException e) {
if (recorder != null) {
recorder.recordErrorEvent(e);
}

throw e;
Comment on lines +23 to +41
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 If the user-supplied NetworkTelemetryRecorder throws an unchecked RuntimeException, two bugs occur in RollbarOkHttpInterceptor.intercept(): (1) in the success path, the Response obtained from chain.proceed() is never returned or closed, leaking OkHttp connections and eventually exhausting the connection pool; (2) in the error path, the original IOException is silently swallowed and the RuntimeException propagates instead, breaking the throws IOException contract and bypassing callers' retry/fallback logic. Both paths should wrap recorder calls in try-catch(Exception) to isolate recorder failures.

Extended reasoning...

Bug 1 — Response body leak (success path, lines 24–34)

In intercept(), chain.proceed(request) acquires a live OkHttp network connection and wraps it in a Response object. When response.code() >= 400 && recorder \!= null, the code calls recorder.recordNetworkEvent(). If that call throws an unchecked RuntimeException (e.g., a NullPointerException inside the user's Rollbar integration, or a serialization failure), the exception is not caught by the catch (IOException e) block. As a result, return response at line 34 is never reached: the caller never receives the Response and can never call response.close(). Per OkHttp's documentation, response bodies must be explicitly closed to return the underlying connection to the connection pool.

Step-by-step proof (Bug 1): Suppose the server returns a 500 and recorder.recordNetworkEvent() throws new RuntimeException("Rollbar unavailable"). (1) chain.proceed(request) returns a Response holding an open connection — (2) response.code() >= 400 is true, so recorder.recordNetworkEvent() is called — (3) the recorder throws RuntimeException — (4) the exception propagates past return response (never executed) and past the catch (IOException e) block (only catches checked exceptions) — (5) the caller receives a RuntimeException with no Response to close — (6) the connection is held indefinitely. Under sustained 4xx/5xx traffic with a broken Rollbar integration, every error response leaks one connection. With OkHttp's default pool of 5 connections, as few as 5 failures can freeze all subsequent HTTP calls.

Bug 2 — IOException masked (error path, lines 36–41)

In the catch (IOException e) block, recorder.recordErrorEvent(e) is called before throw e. If recordErrorEvent() throws a RuntimeException R, the throw e statement is never executed. R propagates to the caller instead of the original IOException. This breaks the method's throws IOException declaration: callers that wrap the call in catch (IOException) for retry or fallback logic will not catch R, the original network failure (timeout, connection refused, etc.) is lost, and async frameworks like Kotlin coroutines or RxJava that classify errors by type will misclassify the failure.

Step-by-step proof (Bug 2): Suppose a DNS failure causes chain.proceed() to throw new IOException("Unable to resolve host"). (1) The catch (IOException e) block is entered — (2) recorder.recordErrorEvent(e) is called, and the recorder internally calls rollbar.log(exception) which throws new NullPointerException() due to a missing payload field — (3) the NPE propagates out of the catch block; throw e is never reached — (4) the caller's catch (IOException) block is bypassed entirely — (5) the original "Unable to resolve host" error is invisible; the caller sees only an NPE with no network context.

Why existing code doesn't prevent it: The catch (IOException e) block only catches checked IOExceptions. Both RuntimeException thrown from recordNetworkEvent() in the try block and RuntimeException thrown from recordErrorEvent() in the catch block are unchecked and propagate freely through these constructs. There is no finally block and no suppression of recorder exceptions.

Fix: Wrap each recorder call in try-catch(Exception) (or at minimum try-catch(RuntimeException)) and log/suppress the recorder's exception. For Bug 1, also add a try-finally (or try-with-resources) around the response to ensure response.close() is called if a recorder exception escapes before return response. The recorder's failure should never affect the interceptor's core behavior.

}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
package com.rollbar.okhttp;

import com.rollbar.api.payload.data.Level;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.Response;
import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
import okhttp3.mockwebserver.SocketPolicy;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import java.io.IOException;

import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.*;

class RollbarOkHttpInterceptorTest {

private MockWebServer server;
private NetworkTelemetryRecorder recorder;
private OkHttpClient client;

@BeforeEach
void setUp() throws IOException {
server = new MockWebServer();
server.start();

recorder = mock(NetworkTelemetryRecorder.class);

client = new OkHttpClient.Builder()
.addInterceptor(new RollbarOkHttpInterceptor(recorder))
.build();
}

@AfterEach
void tearDown() throws IOException {
server.shutdown();
}

@Test
void successfulResponse_doesNotRecordEvent() throws IOException {
server.enqueue(new MockResponse().setResponseCode(200));

Request request = new Request.Builder().url(server.url("/ok")).build();
Response response = client.newCall(request).execute();
response.close();

assertEquals(200, response.code());
verifyNoInteractions(recorder);
}

@Test
void redirectResponse_doesNotRecordEvent() throws IOException {
server.enqueue(new MockResponse().setResponseCode(301).addHeader("Location", "/other"));

OkHttpClient noFollowClient = client.newBuilder().followRedirects(false).build();
Request request = new Request.Builder().url(server.url("/redirect")).build();
Response response = noFollowClient.newCall(request).execute();
response.close();

assertEquals(301, response.code());
verifyNoInteractions(recorder);
}

@Test
void clientErrorResponse_recordsNetworkEvent() throws IOException {
server.enqueue(new MockResponse().setResponseCode(404));

Request request = new Request.Builder().url(server.url("/not-found")).build();
Response response = client.newCall(request).execute();
response.close();

assertEquals(404, response.code());
verify(recorder).recordNetworkEvent(
eq(Level.CRITICAL), eq("GET"), contains("/not-found"), eq("404"));
verify(recorder, never()).recordErrorEvent(any());
}

@Test
void serverErrorResponse_recordsNetworkEvent() throws IOException {
server.enqueue(new MockResponse().setResponseCode(500));

Request request = new Request.Builder().url(server.url("/error")).build();
Response response = client.newCall(request).execute();
response.close();

assertEquals(500, response.code());
verify(recorder).recordNetworkEvent(
eq(Level.CRITICAL), eq("GET"), contains("/error"), eq("500"));
verify(recorder, never()).recordErrorEvent(any());
}

@Test
void connectionFailure_recordsErrorEvent() {
server.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.DISCONNECT_AT_START));

Request request = new Request.Builder().url(server.url("/fail")).build();

assertThrows(IOException.class, () -> client.newCall(request).execute());

verify(recorder).recordErrorEvent(any(IOException.class));
verify(recorder, never()).recordNetworkEvent(any(), any(), any(), any());
}

@Test
void postRequest_recordsCorrectMethod() throws IOException {
server.enqueue(new MockResponse().setResponseCode(500));

Request request = new Request.Builder()
.url(server.url("/post"))
.post(okhttp3.RequestBody.create("body", okhttp3.MediaType.parse("text/plain")))
.build();
Response response = client.newCall(request).execute();
response.close();

verify(recorder).recordNetworkEvent(eq(Level.CRITICAL), eq("POST"), any(), eq("500"));
}

@Test
void nullRecorder_errorResponse_doesNotThrowNPE() throws IOException {
server.enqueue(new MockResponse().setResponseCode(500));

OkHttpClient nullRecorderClient = new OkHttpClient.Builder()
.addInterceptor(new RollbarOkHttpInterceptor(null))
.build();

Request request = new Request.Builder().url(server.url("/error")).build();
Response response = nullRecorderClient.newCall(request).execute();
response.close();

assertEquals(500, response.code());
}

@Test
void nullRecorder_connectionFailure_doesNotThrow() {
server.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.DISCONNECT_AT_START));

OkHttpClient nullRecorderClient = new OkHttpClient.Builder()
.addInterceptor(new RollbarOkHttpInterceptor(null))
.build();

Request request = new Request.Builder().url(server.url("/fail")).build();

assertThrows(IOException.class, () -> nullRecorderClient.newCall(request).execute());
}
}
1 change: 1 addition & 0 deletions settings.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ include(
":rollbar-jakarta-web",
":rollbar-log4j2",
":rollbar-logback",
"rollbar-okhttp",
":rollbar-spring-webmvc",
":rollbar-spring6-webmvc",
":rollbar-spring-boot-webmvc",
Expand Down
Loading