You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by "rajat315315 (via GitHub)" <gi...@apache.org> on 2026/07/23 13:55:18 UTC
[PR] Addded try catch block for Pre, Post processors. [jmeter]
rajat315315 opened a new pull request, #6735:
URL: https://github.com/apache/jmeter/pull/6735
Fixes: #6734
## Description
<!--- Provide a general summary of your changes in the Title above -->
<!--- Describe your changes in detail here -->
- 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
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, tests ran to see how -->
<!--- your change affects other areas of the code, etc. -->
## Types of changes
<!--- What types of changes does your code introduce? Delete as appropriate -->
- Bug fix (non-breaking change which fixes an issue)
## Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [ ] My code follows the [code style][style-guide] of this project.
- [ ] I have updated the documentation accordingly.
[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added try catch block for Pre, Post processors. [jmeter]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on code in PR #6735:
URL: https://github.com/apache/jmeter/pull/6735#discussion_r4066315928
##########
src/core/src/main/java/org/apache/jmeter/threads/JMeterThread.java:
##########
@@ -959,18 +959,29 @@ private static void processAssertion(SampleResult result, Assertion assertion) {
private static void runPostProcessors(List<? extends PostProcessor> extractors) {
for (PostProcessor ex : extractors) {
- TestBeanHelper.prepare((TestElement) ex);
- ex.process();
+ try {
+ if (log.isDebugEnabled()) {
+ log.debug("Running postprocessor: {}", ((AbstractTestElement) ex).getName());
+ }
+ TestBeanHelper.prepare((TestElement) ex);
+ ex.process();
+ } catch (Exception | JMeterError e) {
Review Comment:
`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:
```java
} catch (JMeterStopTestException | JMeterStopTestNowException | JMeterStopThreadException e) {
throw e; // preserve control-flow semantics
} catch (Exception | JMeterError e) {
log.error("Error processing PostProcessor: {}", ((AbstractTestElement) ex).getName(), e);
}
```
##########
src/core/src/test/java/org/apache/jmeter/threads/TestJMeterThread.java:
##########
@@ -168,6 +172,114 @@ void testBug63490EndTestWhenDelayIsTooLongForScheduler() {
assertTrue(duration <= maxDuration, "Test plan should not run for longer than duration");
}
+ @Test
+ void testPostProcessorExceptionHandling() {
+ JMeterContextService.getContext().setVariables(new JMeterVariables());
+
+ HashTree testTree = new HashTree();
+ LoopController samplerController = new LoopController();
+ samplerController.setLoops(1);
+ samplerController.setContinueForever(false);
+ samplerController.setEnabled(true);
+
+ DummySampler dummySampler = createSampler();
+ DummyPostProcessor failingPostProcessor = new DummyPostProcessor(true);
+ DummyPostProcessor secondPostProcessor = new DummyPostProcessor(false);
+
+ HashTree samplerControllerTree = testTree.add(samplerController);
+ HashTree samplerTree = samplerControllerTree.add(dummySampler);
+ samplerTree.add(failingPostProcessor);
+ samplerTree.add(secondPostProcessor);
+
+ ThreadGroup threadGroup = new ThreadGroup();
+ threadGroup.setNumThreads(1);
+
+ JMeterThread jMeterThread = new JMeterThread(testTree, threadGroup, null);
+ jMeterThread.setThreadGroup(threadGroup);
+ jMeterThread.run();
+
+ assertTrue(dummySampler.isCalled(), "Sampler should be executed");
+ assertTrue(failingPostProcessor.isCalled(), "Failing post processor should be executed");
+ assertTrue(secondPostProcessor.isCalled(), "Second post processor should still be executed after exception in first");
+ }
+
+ @Test
+ void testPreProcessorExceptionHandling() {
+ JMeterContextService.getContext().setVariables(new JMeterVariables());
+
+ HashTree testTree = new HashTree();
+ LoopController samplerController = new LoopController();
+ samplerController.setLoops(1);
+ samplerController.setContinueForever(false);
+ samplerController.setEnabled(true);
+
+ DummySampler dummySampler = createSampler();
+ DummyPreProcessor failingPreProcessor = new DummyPreProcessor(true);
+ DummyPreProcessor secondPreProcessor = new DummyPreProcessor(false);
+
+ HashTree samplerControllerTree2 = testTree.add(samplerController);
+ HashTree samplerTree2 = samplerControllerTree2.add(dummySampler);
+ samplerTree2.add(failingPreProcessor);
+ samplerTree2.add(secondPreProcessor);
+
+ ThreadGroup threadGroup = new ThreadGroup();
+ threadGroup.setNumThreads(1);
+
+ JMeterThread jMeterThread = new JMeterThread(testTree, threadGroup, null);
+ jMeterThread.setThreadGroup(threadGroup);
+ jMeterThread.run();
+
+ assertTrue(failingPreProcessor.isCalled(), "Failing pre processor should be executed");
+ assertTrue(secondPreProcessor.isCalled(), "Second pre processor should still be executed after exception in first");
+ assertTrue(dummySampler.isCalled(), "Sampler should still be executed after preprocessor exception");
+ }
+
+ private static class DummyPostProcessor extends AbstractTestElement implements PostProcessor {
+ private static final long serialVersionUID = 1L;
+ private final boolean shouldThrow;
+ private boolean called = false;
+
+ public DummyPostProcessor(boolean shouldThrow) {
+ this.shouldThrow = shouldThrow;
+ setEnabled(true);
+ }
+
+ @Override
+ public void process() {
+ called = true;
+ if (shouldThrow) {
+ throw new RuntimeException("PostProcessor test exception");
+ }
+ }
+
+ public boolean isCalled() {
+ return called;
+ }
+ }
+
+ private static class DummyPreProcessor extends AbstractTestElement implements PreProcessor {
+ private static final long serialVersionUID = 1L;
+ private final boolean shouldThrow;
+ private boolean called = false;
+
+ public DummyPreProcessor(boolean shouldThrow) {
+ this.shouldThrow = shouldThrow;
+ setEnabled(true);
+ }
+
+ @Override
+ public void process() {
+ called = true;
+ if (shouldThrow) {
+ throw new RuntimeException("PreProcessor test exception");
+ }
+ }
+
+ public boolean isCalled() {
+ return called;
+ }
+ }
+
Review Comment:
### 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.
##########
src/core/src/main/java/org/apache/jmeter/threads/JMeterThread.java:
##########
@@ -959,18 +959,29 @@ private static void processAssertion(SampleResult result, Assertion assertion) {
private static void runPostProcessors(List<? extends PostProcessor> extractors) {
for (PostProcessor ex : extractors) {
- TestBeanHelper.prepare((TestElement) ex);
- ex.process();
+ try {
+ if (log.isDebugEnabled()) {
+ log.debug("Running postprocessor: {}", ((AbstractTestElement) ex).getName());
+ }
+ TestBeanHelper.prepare((TestElement) ex);
+ ex.process();
+ } catch (Exception | JMeterError e) {
+ log.error("Error processing PostProcessor: {}", ((AbstractTestElement) ex).getName(), e);
+ }
}
}
private static void runPreProcessors(List<? extends PreProcessor> preProcessors) {
for (PreProcessor ex : preProcessors) {
- if (log.isDebugEnabled()) {
- log.debug("Running preprocessor: {}", ((AbstractTestElement) ex).getName());
+ try {
+ if (log.isDebugEnabled()) {
+ log.debug("Running preprocessor: {}", ((AbstractTestElement) ex).getName());
+ }
+ TestBeanHelper.prepare((TestElement) ex);
+ ex.process();
+ } catch (Exception | JMeterError e) {
Review Comment:
Same comment as above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org