don't try to send more diagnostic events after an unrecoverable HTTP error

Author: eli-darkly

src/main/java/com/launchdarkly/sdk/server/DefaultEventProcessor.java

@@ -262,7 +262,7 @@ private EventDispatcher(
 
  if (diagnosticAccumulator != null) {
  // Set up diagnostics
- this.sendDiagnosticTaskFactory = new SendDiagnosticTaskFactory(eventsConfig);
+ this.sendDiagnosticTaskFactory = new SendDiagnosticTaskFactory(eventsConfig, this::handleResponse);
  sharedExecutor.submit(sendDiagnosticTaskFactory.createSendDiagnosticTask(diagnosticInitEvent));
  } else {
  sendDiagnosticTaskFactory = null;
@@ -334,6 +334,9 @@ private void runMainLoop(BlockingQueue<EventProcessorMessage> inbox,
  }
 
  private void sendAndResetDiagnostics(EventBuffer outbox) {
+ if (disabled.get()) {
+ return;
+ }
  long droppedEvents = outbox.getAndClearDroppedCount();
  // We pass droppedEvents and deduplicatedUsers as parameters here because they are updated frequently in the main loop so we want to avoid synchronization on them.
  DiagnosticEvent diagnosticEvent = diagnosticAccumulator.createEventAndReset(droppedEvents, deduplicatedUsers);
@@ -597,17 +600,26 @@ void stop() {
 
  private static final class SendDiagnosticTaskFactory {
  private final EventsConfiguration eventsConfig;
+ private final EventResponseListener eventResponseListener;
 
- SendDiagnosticTaskFactory(EventsConfiguration eventsConfig) {
+ SendDiagnosticTaskFactory(
+ EventsConfiguration eventsConfig,
+ EventResponseListener eventResponseListener
+ ) {
  this.eventsConfig = eventsConfig;
+ this.eventResponseListener = eventResponseListener;
  }
 
  Runnable createSendDiagnosticTask(final DiagnosticEvent diagnosticEvent) {
  return new Runnable() {
  @Override
  public void run() {
  String json = JsonHelpers.serialize(diagnosticEvent);
- eventsConfig.eventSender.sendEventData(EventDataKind.DIAGNOSTICS, json, 1, eventsConfig.eventsUri);
+ EventSender.Result result = eventsConfig.eventSender.sendEventData(EventDataKind.DIAGNOSTICS,
+ json, 1, eventsConfig.eventsUri);
+ if (eventResponseListener != null) {
+ eventResponseListener.handleResponse(result);
+ }
  }
  };
  }

src/test/java/com/launchdarkly/sdk/server/DefaultEventProcessorDiagnosticsTest.java

@@ -123,18 +123,31 @@ public void periodicDiagnosticEventGetsEventsInLastBatchAndDeduplicatedUsers() t
 
  @Test
  public void periodicDiagnosticEventsAreSentAutomatically() throws Exception {
- // This test overrides the diagnostic recording interval to a small value and verifies that we see
- // at least one periodic event without having to force a send via ep.postDiagnostic().
  MockEventSender es = new MockEventSender();
  DiagnosticId diagnosticId = new DiagnosticId(SDK_KEY);
  ClientContext context = clientContext(SDK_KEY, LDConfig.DEFAULT); 
  DiagnosticEvent.Init initEvent = new DiagnosticEvent.Init(0, diagnosticId, LDConfig.DEFAULT,
  context.getBasic(), context.getHttp());
  DiagnosticAccumulator diagnosticAccumulator = new DiagnosticAccumulator(diagnosticId);
- Duration briefPeriodicInterval = Duration.ofMillis(50);
 
+ EventsConfiguration eventsConfig = makeEventsConfigurationWithBriefDiagnosticInterval(es);
+ 
+ try (DefaultEventProcessor ep = new DefaultEventProcessor(eventsConfig, sharedExecutor, Thread.MAX_PRIORITY,
+ diagnosticAccumulator, initEvent)) {
+ // Ignore the initial diagnostic event
+ es.awaitRequest();
+
+ MockEventSender.Params periodicReq = es.awaitRequest();
+
+ assertNotNull(periodicReq);
+ DiagnosticEvent.Statistics statsEvent = gson.fromJson(periodicReq.data, DiagnosticEvent.Statistics.class);
+ assertEquals("diagnostic", statsEvent.kind);
+ }
+ }
+
+ private EventsConfiguration makeEventsConfigurationWithBriefDiagnosticInterval(EventSender es) {
  // Can't use the regular config builder for this, because it will enforce a minimum flush interval
- EventsConfiguration eventsConfig = new EventsConfiguration(
+ return new EventsConfiguration(
  false,
  100,
  es,
@@ -144,18 +157,34 @@ public void periodicDiagnosticEventsAreSentAutomatically() throws Exception {
  ImmutableSet.of(),
  100,
  Duration.ofSeconds(5),
- briefPeriodicInterval
+ Duration.ofMillis(50)
  );
+ }
+
+ @Test
+ public void diagnosticEventsStopAfter401Error() throws Exception {
+ // This is easier to test with a mock component than it would be in LDClientEndToEndTest, because
+ // we don't have to worry about the latency of a real HTTP request which could allow the periodic
+ // task to fire again before we received a response. In real life, that wouldn't matter because
+ // the minimum diagnostic interval is so long, but in a test we need to be able to use a short
+ // interval.
+ MockEventSender es = new MockEventSender();
+ es.result = new EventSender.Result(false, true, null); // mustShutdown=true; this is what would be returned for a 401 error
+
+ DiagnosticId diagnosticId = new DiagnosticId(SDK_KEY);
+ ClientContext context = clientContext(SDK_KEY, LDConfig.DEFAULT); 
+ DiagnosticEvent.Init initEvent = new DiagnosticEvent.Init(0, diagnosticId, LDConfig.DEFAULT,
+ context.getBasic(), context.getHttp());
+ DiagnosticAccumulator diagnosticAccumulator = new DiagnosticAccumulator(diagnosticId);
+ 
+ EventsConfiguration eventsConfig = makeEventsConfigurationWithBriefDiagnosticInterval(es);
+ 
  try (DefaultEventProcessor ep = new DefaultEventProcessor(eventsConfig, sharedExecutor, Thread.MAX_PRIORITY,
  diagnosticAccumulator, initEvent)) {
  // Ignore the initial diagnostic event
  es.awaitRequest();
 
- MockEventSender.Params periodicReq = es.awaitRequest();
-
- assertNotNull(periodicReq);
- DiagnosticEvent.Statistics statsEvent = gson.fromJson(periodicReq.data, DiagnosticEvent.Statistics.class);
- assertEquals("diagnostic", statsEvent.kind);
+ es.expectNoRequests(Duration.ofMillis(100));
  }
  }