Skip to content

Commit d098cd6

Browse files
committed
feat: downgrade uncaught error log when reconciler handles the error
When a reconciler's updateErrorStatus returns any ErrorStatusUpdateControl other than defaultErrorProcessing(), the error is now considered handled by the reconciler. Native retry (including @GradualRetry exponential backoff) is kept intact, but the framework logs the error on DEBUG level instead of emitting the "Uncaught error during event processing" WARN. This lets a reconciler keep retrying an expected, recoverable condition without producing a continuous stream of WARN messages, while it remains free to log the error at whatever level it wants inside updateErrorStatus. Returning defaultErrorProcessing() preserves the previous behavior, including the warning. - PostExecutionControl carries an errorHandledByReconciler flag - ReconciliationDispatcher marks the exception control as handled instead of rethrowing when a non-default control still wants retry - EventProcessor downgrades the retry-aware and no-retry-configured error logs to DEBUG when the error was handled by the reconciler
1 parent 958d1d6 commit d098cd6

5 files changed

Lines changed: 94 additions & 9 deletions

File tree

docs/content/en/docs/documentation/error-handling-retries.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,16 @@ Retry can be skipped in cases of unrecoverable errors:
108108
ErrorStatusUpdateControl.patchStatus(customResource).withNoRetry();
109109
```
110110

111+
When `updateErrorStatus` returns any `ErrorStatusUpdateControl` other than
112+
`ErrorStatusUpdateControl.defaultErrorProcessing()`, the framework considers the error handled by
113+
the reconciler. In that case the native retry (including the exponential backoff from
114+
`@GradualRetry`) is still performed, but the framework no longer logs the "Uncaught error during
115+
event processing" warning; the error is logged on `DEBUG` level instead. This lets a reconciler keep
116+
retrying an expected, recoverable condition without producing a continuous stream of `WARN` messages,
117+
while it remains free to log the error at whatever level it deems appropriate inside
118+
`updateErrorStatus`. Returning `ErrorStatusUpdateControl.defaultErrorProcessing()` preserves the
119+
default behavior, including the warning.
120+
111121
### Correctness and Automatic Retries
112122

113123
While it is possible to deactivate automatic retries, this is not desirable unless there is a particular reason.

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,9 @@ synchronized void eventProcessingFinished(
297297
&& postExecutionControl.exceptionDuringExecution()
298298
&& (!state.deleteEventPresent() || triggerOnAllEvents())) {
299299
handleRetryOnException(
300-
executionScope, postExecutionControl.getRuntimeException().orElseThrow());
300+
executionScope,
301+
postExecutionControl.getRuntimeException().orElseThrow(),
302+
postExecutionControl.isErrorHandledByReconciler());
301303
return;
302304
}
303305
cleanupOnSuccessfulExecution(executionScope);
@@ -327,10 +329,17 @@ private boolean isTriggerOnAllEventAndDeleteEventPresent(ResourceState state) {
327329
private void logErrorIfNoRetryConfigured(
328330
ExecutionScope<P> executionScope, PostExecutionControl<P> postExecutionControl) {
329331
if (!isRetryConfigured() && postExecutionControl.exceptionDuringExecution()) {
330-
log.error(
331-
"Error during event processing {}",
332-
executionScope,
333-
postExecutionControl.getRuntimeException().orElseThrow());
332+
if (postExecutionControl.isErrorHandledByReconciler()) {
333+
log.debug(
334+
"Error during event processing {}, but was handled by the reconciler",
335+
executionScope,
336+
postExecutionControl.getRuntimeException().orElseThrow());
337+
} else {
338+
log.error(
339+
"Error during event processing {}",
340+
executionScope,
341+
postExecutionControl.getRuntimeException().orElseThrow());
342+
}
334343
}
335344
}
336345

@@ -369,14 +378,16 @@ TimerEventSource<P> retryEventSource() {
369378
* events (received meanwhile retry is in place or already in buffer) instantly or always wait
370379
* according to the retry timing if there was an exception.
371380
*/
372-
private void handleRetryOnException(ExecutionScope<P> executionScope, Exception exception) {
381+
private void handleRetryOnException(
382+
ExecutionScope<P> executionScope, Exception exception, boolean errorHandledByReconciler) {
373383
final var state = getOrInitRetryExecution(executionScope);
374384
var resourceID = state.getId();
375385
boolean eventPresent =
376386
state.eventPresent()
377387
|| (triggerOnAllEvents() && state.isAdditionalEventPresentAfterDeleteEvent());
378388
state.markEventReceived(triggerOnAllEvents());
379-
retryAwareErrorLogging(state.getRetry(), eventPresent, exception, executionScope);
389+
retryAwareErrorLogging(
390+
state.getRetry(), eventPresent, errorHandledByReconciler, exception, executionScope);
380391
metrics.reconciliationFailed(
381392
executionScope.getResource(), state.getRetry(), exception, metricsMetadata);
382393
if (eventPresent) {
@@ -408,9 +419,17 @@ private void handleRetryOnException(ExecutionScope<P> executionScope, Exception
408419
private void retryAwareErrorLogging(
409420
RetryExecution retry,
410421
boolean eventPresent,
422+
boolean errorHandledByReconciler,
411423
Exception exception,
412424
ExecutionScope<P> executionScope) {
413-
if (!retry.isLastAttempt()
425+
if (errorHandledByReconciler) {
426+
// The reconciler already handled the error in updateErrorStatus (and had the chance to log it
427+
// as needed), so the framework only logs it on debug level while still retrying.
428+
log.debug(
429+
"Error during event processing {}, but was handled by the reconciler",
430+
executionScope,
431+
exception);
432+
} else if (!retry.isLastAttempt()
414433
&& exception instanceof KubernetesClientException ex
415434
&& ex.getCode() == HttpURLConnection.HTTP_CONFLICT) {
416435
log.debug("Full client conflict error during event processing {}", executionScope, exception);

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/PostExecutionControl.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ final class PostExecutionControl<R extends HasMetadata> {
2727
private final Exception runtimeException;
2828

2929
private Long reScheduleDelay = null;
30+
private boolean errorHandledByReconciler = false;
3031

3132
private PostExecutionControl(
3233
boolean finalizerRemoved,
@@ -68,6 +69,22 @@ public static <R extends HasMetadata> PostExecutionControl<R> exceptionDuringExe
6869
return new PostExecutionControl<>(false, null, false, exception);
6970
}
7071

72+
/**
73+
* Marks that the exception was handled by the reconciler's {@code updateErrorStatus} (i.e. the
74+
* reconciler did not return {@link
75+
* io.javaoperatorsdk.operator.api.reconciler.ErrorStatusUpdateControl#defaultErrorProcessing()}),
76+
* but the error is still retried. In this case the framework logs the error on a lower level,
77+
* since the reconciler already had the chance to handle and log it as needed.
78+
*/
79+
public PostExecutionControl<R> withErrorHandledByReconciler() {
80+
this.errorHandledByReconciler = true;
81+
return this;
82+
}
83+
84+
public boolean isErrorHandledByReconciler() {
85+
return errorHandledByReconciler;
86+
}
87+
7188
public Optional<R> getUpdatedCustomResource() {
7289
return Optional.ofNullable(updatedCustomResource);
7390
}

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,12 @@ public boolean isLastAttempt() {
266266
errorStatusUpdateControl.getScheduleDelay().ifPresent(postExecutionControl::withReSchedule);
267267
return postExecutionControl;
268268
}
269-
throw e;
269+
// The reconciler handled the error via updateErrorStatus (it did not return
270+
// defaultErrorProcessing()) but still wants the error to be retried. The retry (and its
271+
// backoff) is kept intact, but since the reconciler already had the chance to handle and log
272+
// the error, the framework logs it on a lower level instead of emitting an "uncaught error"
273+
// warning.
274+
return PostExecutionControl.<P>exceptionDuringExecution(e).withErrorHandledByReconciler();
270275
}
271276

272277
private PostExecutionControl<P> createPostExecutionControl(

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,40 @@ void callErrorStatusHandlerEvenOnFirstError() {
481481
assertThat(postExecControl.exceptionDuringExecution()).isTrue();
482482
}
483483

484+
@Test
485+
void errorHandledByReconcilerMarkedWhenErrorStatusHandledButRetried() {
486+
testCustomResource.addFinalizer(DEFAULT_FINALIZER);
487+
reconciler.reconcile =
488+
(r, c) -> {
489+
throw new IllegalStateException("Error Status Test");
490+
};
491+
reconciler.errorHandler = () -> ErrorStatusUpdateControl.patchStatus(testCustomResource);
492+
493+
var postExecControl =
494+
reconciliationDispatcher.handleExecution(
495+
new ExecutionScope(null, null, false, false).setResource(testCustomResource));
496+
497+
assertThat(postExecControl.exceptionDuringExecution()).isTrue();
498+
assertThat(postExecControl.isErrorHandledByReconciler()).isTrue();
499+
}
500+
501+
@Test
502+
void errorNotHandledByReconcilerOnDefaultErrorProcessing() {
503+
testCustomResource.addFinalizer(DEFAULT_FINALIZER);
504+
reconciler.reconcile =
505+
(r, c) -> {
506+
throw new IllegalStateException("Error Status Test");
507+
};
508+
reconciler.errorHandler = () -> ErrorStatusUpdateControl.defaultErrorProcessing();
509+
510+
var postExecControl =
511+
reconciliationDispatcher.handleExecution(
512+
new ExecutionScope(null, null, false, false).setResource(testCustomResource));
513+
514+
assertThat(postExecControl.exceptionDuringExecution()).isTrue();
515+
assertThat(postExecControl.isErrorHandledByReconciler()).isFalse();
516+
}
517+
484518
@Test
485519
void errorHandlerCanInstructNoRetryWithUpdate() {
486520
testCustomResource.addFinalizer(DEFAULT_FINALIZER);

0 commit comments

Comments
 (0)