AS-5201: include topic name in all exceptions by Rutuja-IBM · Pull Request #198 · datastax/pulsar-jms

Rutuja-IBM · 2026-04-09T06:54:36Z

No description provided.

heesung-sohn · 2026-04-13T18:24:01Z

do we have any racing condition that a thread could handle different topics ? I am worried that this thread local topic could wrongly print topic name while handling other topics.

If there is any async operation(perhaps this can be introduced in the future) in the try/finally block, the thread context lifecycle management will become more complex, and it would be hard to trace.

I think we better just directly pass topicName in the target code block (from the stack) without this thread local context.

plz update the description to explain more about this change.

Rutuja-IBM · 2026-04-14T11:20:09Z

Instead of using thread-local to store the topic, we now pass the topic name directly where the code runs. This avoids confusion and wrong values when multiple threads are involved.

Created a common method to handle all exceptions.

Updated Utils.java: Modified method signature to handleException(Throwable cause, String topic) to include topic context.

Updated all calling classes to pass the topic as an argument wherever handleException is used.
When available, the topic is passed (topic name, fully qualified domain name, or Pulsar topic).
When not available or not required, "null" is passed.

This method:
finds the real error, converts it into a JMSException, and adds the topic name to the error message.

Also added wrapper methods so that: we don’t need to write try-catch everywhere, and all errors automatically go through this common handling.

heesung-sohn · 2026-04-14T20:52:38Z

@@ -53,25 +53,43 @@
 public final class Utils {
  private Utils() {}



Add this overloaded func and use this func if topic is unknown, instead of directly calling handleException(e, null),

public static JMSException handleException(Throwable cause) { return handleException(cause, null); }

heesung-sohn · 2026-04-14T20:53:16Z


 final class ConsumerConfigurationTest {
+  @RegisterExtension
+  static PulsarContainerExtension pulsarContainer = new PulsarContainerExtension();


why this is required? remove it

heesung-sohn · 2026-04-14T20:53:27Z

    <dependency>
      <groupId>org.testcontainers</groupId>
      <artifactId>junit-jupiter</artifactId>
+      <version>${testcontainers.version}</version>


heesung-sohn · 2026-04-14T20:53:36Z

        </executions>
      </plugin>
+      <plugin>
+        <artifactId>maven-assembly-plugin</artifactId>


heesung-sohn · 2026-04-14T20:56:04Z

+            try {
+              return code.getAsBoolean();
+            } catch (Throwable err) {
+              throw handleException(err, topic);


why adding try/catch here?

heesung-sohn · 2026-04-14T20:56:23Z

+    return buildMessage(safeMsg(t), topic);
+  }
+
+  public static <T> T executeWithTopic(String topic, SupplierWithException<T> code)


remove funcs if not required.

heesung-sohn · 2026-04-14T21:03:11Z

-                        if (message == null) {
-                          return null;
+    String topic = destination != null ? destination.getName() : null;
+    try {


why add try?

heesung-sohn · 2026-04-14T21:03:48Z

+    PulsarDestination dest = PulsarConnectionFactory.toPulsarDestination(destination);
+    String topicName = factory.getPulsarTopicName(dest);
+    String subscriptionName = factory.getQueueSubscriptionName(dest);
    try {


why move try?

heesung-sohn · 2026-04-14T21:03:53Z

      throws JMSException {
+    PulsarDestination dest = PulsarConnectionFactory.toPulsarDestination(destination);
+    String topicName = factory.getPulsarTopicName(dest);
    try {


why move try?

heesung-sohn · 2026-04-14T21:04:31Z

-    validateSelector(enableFilters, selector);
+    PulsarDestination dest = PulsarConnectionFactory.toPulsarDestination(destination);
+    String topicName = factory.getPulsarTopicName(dest);
    try {


why move try?

heesung-sohn · 2026-04-14T21:04:47Z

+      throw Utils.handleException(error, topicName);
+
+    } catch (Throwable t) {
+      throw Utils.handleException(t, topicName);


heesung-sohn · 2026-04-14T21:06:29Z

    <maven.antrun.plugin.version>3.1.0</maven.antrun.plugin.version>
    <snakeyaml.version>2.0</snakeyaml.version>
-    <testcontainers.version>1.20.4</testcontainers.version>
+    <testcontainers.version>1.21.4</testcontainers.version>


why change here? revert it this is not the scope in this PR

heesung-sohn · 2026-04-14T21:08:33Z

plz not move try/catch lines.
dont add additional catch unless required.

heesung-sohn

And lets not cover all exception cases in this PR.

Lets scope this PR to the specific case that the customer is complaining. Can you pinpoint the code path according the customer error logs?

In general, pulsar client tries to print topic name from its exception.

Rutuja-IBM added 2 commits April 9, 2026 12:14

Context wrapping for exception

47e7d59

removed code block

886fee5

Rutuja-IBM changed the title ~~As 5201 include topic name in all exceptions~~ AS-5201: include topic name in all exceptions Apr 9, 2026

hshankar31 reviewed Apr 13, 2026

View reviewed changes

Comment thread pulsar-jms/src/main/java/com/datastax/oss/pulsar/jms/Utils.java

[fix] passing topicName in the target code block

14c23ee

Rutuja-IBM requested review from dlg99, heesung-sohn, hshankar31 and sandeep-ctds April 14, 2026 11:20

heesung-sohn reviewed Apr 14, 2026

View reviewed changes

heesung-sohn reviewed Apr 15, 2026

View reviewed changes

Rutuja-IBM mentioned this pull request Apr 15, 2026

AS-5201: Include topic name in required exception #200

Open

		@@ -53,25 +53,43 @@
		public final class Utils {
		private Utils() {}

Conversation

Rutuja-IBM commented Apr 9, 2026

Uh oh!

Uh oh!

heesung-sohn commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Rutuja-IBM commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

heesung-sohn commented Apr 14, 2026

Uh oh!

heesung-sohn left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

heesung-sohn commented Apr 13, 2026 •

edited

Loading

Rutuja-IBM commented Apr 14, 2026 •

edited

Loading