Skip to content

fix(parseq): add warning log on exception in withSafeSideEffect (#326) - #359

Open
reginaldalfret wants to merge 1 commit into
linkedin:masterfrom
reginaldalfret:fix/safe-side-effect-logging
Open

reginaldalfret wants to merge 1 commit into
linkedin:masterfrom
reginaldalfret:fix/safe-side-effect-logging

Conversation

@reginaldalfret

Copy link
Copy Markdown

Summary

This PR addresses and resolves #326 (Proper logging for withSafeSideEffect).

Problem

In Task.withSafeSideEffect(...), any Throwable thrown during the evaluation of the task provider function (Function1) or callable (Callable) was caught and wrapped into a Task.failure(desc, t) without emitting any log message:

      try {
        Task<?> task = func.apply(param);
        if (task == null) {
          throw new RuntimeException(desc + " returned null");
        }
        return task;
      } catch (Throwable t) {
        return Task.failure(desc, t);
      }

Because withSafeSideEffect is specifically designed to isolate side-effect failures and prevent them from failing the parent execution plan, exceptions thrown during task instantiation failed completely silently without trace in application logs. Callers had no straightforward way to diagnose or debug why a side effect was not created.

Changes

  1. Warning Log Output:
    • Added LOGGER.warn("An exception was thrown while creating safe side effect task: " + desc, t); in both the instance withSafeSideEffect(final String desc, final Function1<? super T, Task<?>> func) and static withSafeSideEffect(final String desc, final Callable<Task<T>> func) methods before returning Task.failure(desc, t).
  2. Whitespace Correction:
    • Fixed missing space in static withSafeSideEffect: "returned null" -> " returned null", matching instance withSafeSideEffect.
  3. Regression Tests:
    • Updated testWithSafeSideEffectFailureInTaskProviderFunction and testStaticWithSafeSideEffectFailureInTaskProviderFunction in AbstractTaskTest.java to attach a TestLogAppender and assert that a WARN level log entry is emitted with the corresponding side-effect description and exception type.

Validation

  • Compiled and executed the test suite with Gradle 6.3 on OpenJDK 11:
    • :parseq:test --tests "com.linkedin.parseq.TestTask": 47 passed, 0 failures, 0 errors, 1 skipped (historical).
    • All log assertions verified.

Fixes #326.

Signed-off-by: Reginald Alfret reginaldalfret@gmail.com

…edin#326)

When an exception is thrown inside the task provider function or callable
supplied to withSafeSideEffect, the exception was caught and converted into a
failed task without any log entry. This made side effect failures difficult to
diagnose and observe from the caller side.

This fix:
1. Adds a warning log via LOGGER.warn before returning Task.failure in both
   instance and static variants of withSafeSideEffect.
2. Fixes missing whitespace in 'desc + " returned null"' for static withSafeSideEffect.
3. Adds regression tests validating that warning log events are generated with the
   expected description and exception class when side effect creation fails.

Resolves linkedin#326

Signed-off-by: Reginald Alfret <reginaldalfret@gmail.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proper logging for withSafeSideEffect

1 participant