Skip to content

Conversation

@thomas-bousquet
Copy link
Contributor

@thomas-bousquet thomas-bousquet commented Dec 1, 2025

Motivation

When the Pulsar client is created, if EnableTransaction is true then the transaction coordinator client is created.

If an application tries to create a transaction with NewTransaction() when EnableTransaction is false, the application will panic; returning an error when the transaction coordinator is nil will prevent this.

Modifications

In NewTransaction(), a nil check is added . If the transaction coordination client is nil, the method now returns an error.

Verifying this change

  • Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): no
  • The public API: no
  • The schema: no
  • The default values of configurations: no
  • The wire protocol: no

Documentation

  • Does this pull request introduce a new feature? no

@crossoverJie
Copy link
Member

Please add a unit test.

@thomas-bousquet
Copy link
Contributor Author

thomas-bousquet commented Dec 2, 2025

Please add a unit test.

I added a test in the integration tests suite for transactions. I also exported the error, as it could be leveraged with errors.Is() by the applications creating the client.

@nodece nodece merged commit 6ab93d5 into apache:master Dec 3, 2025
7 checks passed
@crossoverJie crossoverJie added this to the v0.18.0 milestone Dec 3, 2025
RobertIndie pushed a commit that referenced this pull request Dec 8, 2025
#1444)

* fix: return error when the client transaction coordinator is nil to prevent panic

* test: add testcase to ensure error is actually returned

(cherry picked from commit 6ab93d5)
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.

3 participants