diff --git a/backlog/tasks/task-208 - Harden-analytics-tracking-for-offline-fire-and-forget-behavior.md b/backlog/tasks/task-208 - Harden-analytics-tracking-for-offline-fire-and-forget-behavior.md new file mode 100644 index 00000000..136e42a6 --- /dev/null +++ b/backlog/tasks/task-208 - Harden-analytics-tracking-for-offline-fire-and-forget-behavior.md @@ -0,0 +1,66 @@ +--- +id: TASK-208 +title: Harden analytics tracking for offline fire-and-forget behavior +status: Done +assignee: + - codex +created_date: '2026-04-13 12:25' +updated_date: '2026-04-13 12:44' +labels: + - analytics + - stability +dependencies: [] +references: + - src/main/java/com/devoxx/genie/service/analytics/AnalyticsService.java + - src/test/java/com/devoxx/genie/service/analytics/AnalyticsServiceTest.java + - src/main/java/com/devoxx/genie/service/prompt/PromptExecutionService.java + - src/main/java/com/devoxx/genie/ui/window/DevoxxGenieToolWindowContent.java +priority: high +--- + +## Description + + +Ensure anonymous usage analytics can never crash or interrupt the IntelliJ plugin when the user is offline, the analytics endpoint is unreachable, or analytics setup fails. Tracking must remain non-critical, fire-and-forget behavior from every public analytics entry point. + + +## Acceptance Criteria + +- [x] #1 Public analytics tracking methods never throw to callers when state access, payload creation, scheduling, URI creation, or network delivery fails +- [x] #2 Analytics HTTP delivery remains asynchronous in production and never blocks the EDT or prompt/model-selection flow +- [x] #3 Offline, DNS, timeout, and non-2xx endpoint failures are swallowed and logged only at debug level +- [x] #4 Regression tests cover silent failure before scheduling and during network delivery + + +## Implementation Plan + + +1. Harden `AnalyticsService` public tracking entry points so analytics preconditions, state lookup, payload construction, endpoint parsing, client creation, and dispatch failures cannot propagate to callers. +2. Use asynchronous `HttpClient.sendAsync` for production delivery so analytics remains fire-and-forget without occupying IntelliJ pooled threads while offline or timing out. +3. Keep test-only synchronous injection for deterministic existing tests, and add async test coverage for failed delivery. +4. Update analytics call sites where needed so service lookup/tracking remains non-critical. +5. Run the focused analytics test class and update acceptance criteria based on verified behavior. + + +## Implementation Notes + + +Implemented analytics hardening in the plugin: production delivery now uses HttpClient.sendAsync, public tracking methods catch pre-send failures, and call sites use safe static entry points so service lookup/tracking remains non-critical. Added regression coverage for state lookup failure, invalid endpoint URI, synchronous network failure, and async network failure. Verified with `./gradlew -q test --tests com.devoxx.genie.service.analytics.AnalyticsServiceTest`. + +Added explicit regression coverage for non-2xx analytics responses remaining silent, and reran `./gradlew -q test --tests com.devoxx.genie.service.analytics.AnalyticsServiceTest` successfully. + + +## Final Summary + + +Summary: +- Hardened `AnalyticsService` so analytics precondition checks, state lookup, endpoint parsing, request dispatch, and HTTP failures are swallowed and logged at debug level instead of propagating to plugin callers. +- Switched production analytics delivery from IntelliJ pooled-thread blocking sends to `HttpClient.sendAsync`, keeping prompt/model-selection flows fire-and-forget when offline or when the endpoint is unreachable. +- Updated prompt execution and model-selection call sites to use safe analytics entry points. +- Added regression tests for state lookup failure, invalid endpoint URI, synchronous network failure, async network failure, and non-2xx endpoint responses. + +Tests: +- `./gradlew -q test --tests com.devoxx.genie.service.analytics.AnalyticsServiceTest` + +PR: https://github.com/devoxx/DevoxxGenieIDEAPlugin/pull/1007 + diff --git a/src/main/java/com/devoxx/genie/service/analytics/AnalyticsService.java b/src/main/java/com/devoxx/genie/service/analytics/AnalyticsService.java index 1081a4f1..5c76e5ed 100644 --- a/src/main/java/com/devoxx/genie/service/analytics/AnalyticsService.java +++ b/src/main/java/com/devoxx/genie/service/analytics/AnalyticsService.java @@ -16,6 +16,7 @@ import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.time.Duration; +import java.util.concurrent.CompletionException; import java.util.concurrent.ThreadLocalRandom; /** @@ -28,8 +29,8 @@ * text, response text, conversation history, file content, file paths, project names, API keys, * or user identity is ever sent. * - *

Calls are fire-and-forget on the application thread pool and never block the EDT. Failures - * are logged at debug level and never surfaced to the user. + *

Calls are fire-and-forget through {@link HttpClient#sendAsync(HttpRequest, HttpResponse.BodyHandler)} + * and never block the EDT. Failures are logged at debug level and never surfaced to the user. */ @Slf4j @Service(Service.Level.APP) @@ -53,12 +54,36 @@ public static AnalyticsService getInstance() { return ApplicationManager.getApplication().getService(AnalyticsService.class); } + public static void trackPromptExecutedSafely(@Nullable String providerId, @Nullable String modelName) { + try { + getInstance().trackPromptExecuted(providerId, modelName); + } catch (Exception e) { + logAnalyticsFailure("Analytics tracking skipped", e); + } + } + + public static void trackModelSelectedSafely(@Nullable String providerId, @Nullable String modelName) { + try { + getInstance().trackModelSelected(providerId, modelName); + } catch (Exception e) { + logAnalyticsFailure("Analytics tracking skipped", e); + } + } + public void trackPromptExecuted(@Nullable String providerId, @Nullable String modelName) { - send(EVENT_PROMPT_EXECUTED, providerId, modelName); + sendSafely(EVENT_PROMPT_EXECUTED, providerId, modelName); } public void trackModelSelected(@Nullable String providerId, @Nullable String modelName) { - send(EVENT_MODEL_SELECTED, providerId, modelName); + sendSafely(EVENT_MODEL_SELECTED, providerId, modelName); + } + + private void sendSafely(@NotNull String eventName, @Nullable String providerId, @Nullable String modelName) { + try { + send(eventName, providerId, modelName); + } catch (Exception e) { + logAnalyticsFailure("Analytics tracking skipped", e); + } } private void send(@NotNull String eventName, @Nullable String providerId, @Nullable String modelName) { @@ -86,29 +111,65 @@ private void send(@NotNull String eventName, @Nullable String providerId, @Nulla String payload = buildPayload(clientId, eventName, providerId, modelName); if (synchronousForTest) { - postSilently(endpoint, payload); + postBlockingSilently(endpoint, payload); } else { - ApplicationManager.getApplication().executeOnPooledThread(() -> postSilently(endpoint, payload)); + postAsyncSilently(endpoint, payload); } } - private void postSilently(@NotNull String endpoint, @NotNull String payload) { + private void postAsyncSilently(@NotNull String endpoint, @NotNull String payload) { try { - HttpRequest request = HttpRequest.newBuilder() - .uri(URI.create(endpoint)) - .timeout(Duration.ofSeconds(5)) - .header("Content-Type", "application/json") - .POST(HttpRequest.BodyPublishers.ofString(payload)) - .build(); + HttpRequest request = buildRequest(endpoint, payload); + client().sendAsync(request, HttpResponse.BodyHandlers.discarding()) + .thenAccept(this::logUnexpectedStatus) + .exceptionally(e -> { + logAnalyticsFailure("Analytics post failed", unwrapCompletionException(e)); + return null; + }); + } catch (Exception e) { + logAnalyticsFailure("Analytics post failed", e); + } + } + + private void postBlockingSilently(@NotNull String endpoint, @NotNull String payload) { + try { + HttpRequest request = buildRequest(endpoint, payload); HttpResponse response = client().send(request, HttpResponse.BodyHandlers.discarding()); - if (response.statusCode() / 100 != 2) { - log.debug("Analytics endpoint returned {}", response.statusCode()); - } + logUnexpectedStatus(response); } catch (Exception e) { - log.debug("Analytics post failed: {}", e.getMessage()); + logAnalyticsFailure("Analytics post failed", e); + } + } + + @NotNull + private HttpRequest buildRequest(@NotNull String endpoint, @NotNull String payload) { + return HttpRequest.newBuilder() + .uri(URI.create(endpoint)) + .timeout(Duration.ofSeconds(5)) + .header("Content-Type", "application/json") + .POST(HttpRequest.BodyPublishers.ofString(payload)) + .build(); + } + + private void logUnexpectedStatus(@NotNull HttpResponse response) { + if (response.statusCode() / 100 != 2) { + log.debug("Analytics endpoint returned {}", response.statusCode()); } } + private static void logAnalyticsFailure(@NotNull String prefix, @NotNull Throwable throwable) { + String message = throwable.getMessage(); + log.debug("{}: {}", prefix, message != null ? message : throwable.getClass().getSimpleName()); + } + + @NotNull + private static Throwable unwrapCompletionException(@NotNull Throwable throwable) { + if (throwable instanceof CompletionException && throwable.getCause() != null) { + return throwable.getCause(); + } + return throwable; + } + String buildPayload(@NotNull String clientId, @NotNull String eventName, @NotNull String providerId, @@ -199,7 +260,12 @@ private synchronized HttpClient client() { @TestOnly synchronized void setHttpClientForTest(@Nullable HttpClient client) { + setHttpClientForTest(client, true); + } + + @TestOnly + synchronized void setHttpClientForTest(@Nullable HttpClient client, boolean synchronousForTest) { this.httpClient = client; - this.synchronousForTest = true; + this.synchronousForTest = synchronousForTest; } } diff --git a/src/main/java/com/devoxx/genie/service/prompt/PromptExecutionService.java b/src/main/java/com/devoxx/genie/service/prompt/PromptExecutionService.java index 400d23a6..f9b0837f 100644 --- a/src/main/java/com/devoxx/genie/service/prompt/PromptExecutionService.java +++ b/src/main/java/com/devoxx/genie/service/prompt/PromptExecutionService.java @@ -147,7 +147,7 @@ private void trackPromptExecuted(@NotNull ChatMessageContext context) { if (model == null || model.getProvider() == null) { return; } - AnalyticsService.getInstance().trackPromptExecuted( + AnalyticsService.trackPromptExecutedSafely( model.getProvider().getName(), model.getModelName()); } catch (Exception e) { diff --git a/src/main/java/com/devoxx/genie/ui/window/DevoxxGenieToolWindowContent.java b/src/main/java/com/devoxx/genie/ui/window/DevoxxGenieToolWindowContent.java index 3e0f9ca1..5219d45c 100644 --- a/src/main/java/com/devoxx/genie/ui/window/DevoxxGenieToolWindowContent.java +++ b/src/main/java/com/devoxx/genie/ui/window/DevoxxGenieToolWindowContent.java @@ -295,7 +295,7 @@ private void processModelNameSelection(@NotNull ActionEvent e) { if (!suppressModelSelectionTracking && !llmProviderPanel.isUpdatingModelNames() && selectedModel.getProvider() != null) { - AnalyticsService.getInstance().trackModelSelected( + AnalyticsService.trackModelSelectedSafely( selectedModel.getProvider().getName(), selectedModel.getModelName()); } diff --git a/src/test/java/com/devoxx/genie/service/analytics/AnalyticsServiceTest.java b/src/test/java/com/devoxx/genie/service/analytics/AnalyticsServiceTest.java index c413d02a..6811b2c7 100644 --- a/src/test/java/com/devoxx/genie/service/analytics/AnalyticsServiceTest.java +++ b/src/test/java/com/devoxx/genie/service/analytics/AnalyticsServiceTest.java @@ -13,10 +13,12 @@ import java.util.List; import java.util.Set; import java.util.UUID; +import java.util.concurrent.CompletableFuture; import java.util.regex.Matcher; import java.util.regex.Pattern; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; @@ -146,6 +148,54 @@ void networkFailureIsSilent() { assertThat(httpClient.requestCount()).isOne(); } + @Test + void stateLookupFailureIsSilent() { + try (MockedStatic mocked = mockStatic(DevoxxGenieStateService.class)) { + mocked.when(DevoxxGenieStateService::getInstance).thenThrow(new IllegalStateException("settings unavailable")); + + assertThatCode(() -> service.trackPromptExecuted("anthropic", "claude")) + .doesNotThrowAnyException(); + } + + assertThat(httpClient.requestCount()).isZero(); + } + + @Test + void invalidEndpointIsSilent() { + state.setAnalyticsEndpoint("://not-a-uri"); + + runWithState(() -> + assertThatCode(() -> service.trackPromptExecuted("anthropic", "claude")) + .doesNotThrowAnyException()); + + assertThat(httpClient.requestCount()).isZero(); + } + + @Test + void asyncNetworkFailureIsSilent() { + service.setHttpClientForTest(httpClient, false); + httpClient.throwOnSend = true; + + runWithState(() -> + assertThatCode(() -> service.trackModelSelected("anthropic", "claude")) + .doesNotThrowAnyException()); + + httpClient.awaitOne(); + assertThat(httpClient.requestCount()).isOne(); + } + + @Test + void nonSuccessfulResponseIsSilent() { + httpClient.statusCode = 503; + + runWithState(() -> + assertThatCode(() -> service.trackPromptExecuted("anthropic", "claude")) + .doesNotThrowAnyException()); + + httpClient.awaitOne(); + assertThat(httpClient.requestCount()).isOne(); + } + @Test void payloadContainsNoPiiEvenWhenInputsLookLikePaths() { // A defensive test: even if we ever pass something path-like, the payload only carries @@ -170,6 +220,7 @@ private static class RecordingHttpClient extends HttpClient { private final List requests = new ArrayList<>(); private final List bodies = new ArrayList<>(); boolean throwOnSend = false; + int statusCode = 204; synchronized int requestCount() { return requests.size(); @@ -229,7 +280,7 @@ public synchronized HttpResponse send(HttpRequest request, } @SuppressWarnings("unchecked") HttpResponse stub = (HttpResponse) mock(HttpResponse.class); - when(stub.statusCode()).thenReturn(204); + when(stub.statusCode()).thenReturn(statusCode); return stub; } @@ -242,11 +293,15 @@ public synchronized HttpResponse send(HttpRequest request, @Override public java.util.Optional authenticator() { return java.util.Optional.empty(); } @Override public Version version() { return Version.HTTP_1_1; } @Override public java.util.Optional executor() { return java.util.Optional.empty(); } - @Override public java.util.concurrent.CompletableFuture> sendAsync(HttpRequest request, HttpResponse.BodyHandler responseBodyHandler) { - return java.util.concurrent.CompletableFuture.failedFuture(new UnsupportedOperationException()); + @Override public CompletableFuture> sendAsync(HttpRequest request, HttpResponse.BodyHandler responseBodyHandler) { + try { + return CompletableFuture.completedFuture(send(request, responseBodyHandler)); + } catch (Exception e) { + return CompletableFuture.failedFuture(e); + } } - @Override public java.util.concurrent.CompletableFuture> sendAsync(HttpRequest request, HttpResponse.BodyHandler responseBodyHandler, HttpResponse.PushPromiseHandler pushPromiseHandler) { - return java.util.concurrent.CompletableFuture.failedFuture(new UnsupportedOperationException()); + @Override public CompletableFuture> sendAsync(HttpRequest request, HttpResponse.BodyHandler responseBodyHandler, HttpResponse.PushPromiseHandler pushPromiseHandler) { + return sendAsync(request, responseBodyHandler); } } }