Skip to content

Added try catch block for Pre, Post processors. - #6735

Open
rajat315315 wants to merge 1 commit into
apache:masterfrom
rajat315315:fix/processor-exception-handling
Open

rajat315315 wants to merge 1 commit into
apache:masterfrom
rajat315315:fix/processor-exception-handling

Conversation

@rajat315315

@rajat315315 rajat315315 commented Jul 23, 2026

Copy link
Copy Markdown

Fixes: #6734

Description

  • Updated runPostProcessors(...) in org.apache.jmeter.threads.JMeterThread:
    • Wrapped ex.process() calls in try-catch (Exception | JMeterError e) blocks.
    • Logged errors with the element's name while preserving loop continuity.
  • Updated runPreProcessors(...) in org.apache.jmeter.threads.JMeterThread:
    • Wrapped ex.process() calls in try-catch (Exception | JMeterError e) blocks to ensure pre-processor failures do not interrupt subsequent pre-processors or sample sampling.
  • Added comprehensive unit tests in org.apache.jmeter.threads.TestJMeterThread:
    • testPostProcessorExceptionHandling(): Verifies that when a PostProcessor throws an exception, remaining PostProcessors and sample pipeline steps continue executing.
    • testPreProcessorExceptionHandling(): Verifies that when a PreProcessor throws an exception, remaining PreProcessors and the main sampler execute cleanly.

Motivation and Context

Resolves an issue where an uncaught RuntimeException in a PreProcessor or PostProcessor would abort the execution loop, preventing subsequent processors from running and skipping sample assertions, listener notifications, and compiler state cleanup.

How Has This Been Tested?

Ran Gradle test suite for core module:

./gradlew :src:core:test --tests "org.apache.jmeter.threads.TestJMeterThread"

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the code style of this project.
  • I have updated the documentation accordingly.

@milamberspace milamberspace changed the title Addded try catch block for Pre, Post processors. Added try catch block for Pre, Post processors. Sep 9, 2026

@ruthst00 ruthst00 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Design Concern: Silently Hiding Failures

The current behaviour — letting an exception propagate up to processSampler() — means the sampler result is still recorded and the error is visible in the test results. With this change, a broken post-processor silently does nothing and the test continues as if everything is fine. This could mask real configuration or scripting errors from users.

Consider whether this should instead:

  • Mark the sample result as failed/errored when a post-processor throws, or
  • At minimum, be a configurable/opt-in behaviour rather than always-on.

The existing processAssertion() method handles this well as a reference: it catches exceptions but still records an assertion failure on the result. A similar approach for post-processors would be more transparent.

}
TestBeanHelper.prepare((TestElement) ex);
ex.process();
} catch (Exception | JMeterError e) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

JMeterStopTestException, JMeterStopTestNowException, and JMeterStopThreadException all extend RuntimeException. The new catch (Exception | JMeterError e) block will silently swallow these exceptions, completely breaking the "Stop Test", "Stop Test Now", and "Stop Thread" control-flow mechanisms when they are thrown from inside a pre- or post-processor.

The existing code in processSampler() and run() carefully catches these three exception types before the generic Exception catch, precisely to handle them correctly. The new code does the opposite — it catches Exception first and logs it as an error, discarding the intent entirely.

Fix required: Re-throw these control-flow exceptions before logging the generic error:

} catch (JMeterStopTestException | JMeterStopTestNowException | JMeterStopThreadException e) {
    throw e; // preserve control-flow semantics
} catch (Exception | JMeterError e) {
    log.error("Error processing PostProcessor: {}", ((AbstractTestElement) ex).getName(), e);
}

}
TestBeanHelper.prepare((TestElement) ex);
ex.process();
} catch (Exception | JMeterError e) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same comment as above.

return called;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Test Issues

  1. DummySampler.sample() change is a side-effect: The PR changes the existing DummySampler.sample() to return a non-null SampleResult (previously returned null). This is needed for the new tests to work (so post-processors are actually invoked), but it silently changes the behaviour of the existing testBug63490EndTestWhenDelayIsTooLongForScheduler test. That test asserts assertFalse(dummySampler.isCalled()) — the sampler is never reached due to the timer, so the change doesn't break it, but it's a fragile coupling. The new DummySampler variant used in the new tests should be a separate inner class (or the existing one should be left unchanged and a new subclass created for the new tests).

  2. Variable naming: samplerControllerTree2 in testPreProcessorExceptionHandling() uses a 2 suffix that is a copy-paste artifact from the post-processor test. Since it's a local variable in its own method, it should just be named samplerControllerTree.

  3. Missing import ordering: The two new imports (PostProcessor, PreProcessor) are inserted between ThreadListener and Timer, breaking the alphabetical ordering that the rest of the import block follows. They should be placed before ThreadListener.

  4. No assertion on LAST_SAMPLE_OK: The tests verify that processors and samplers are called, but don't verify that JMeterThread.LAST_SAMPLE_OK is set correctly after a processor failure. Given that the change affects error handling in the sampling pipeline, this would strengthen the test coverage.

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.

[BUG] Unhandled RuntimeException in PreProcessor or PostProcessor aborts remaining processors, assertions, and cleanup

2 participants