Skip to content

Commit 2915dce

Browse files
committed
It's valid to abort a txn that hasn't been started
There may be cases where dependant code starts a transaction using the producer's `#transaction` block syntax and then an exception is raised within that block *before* the transaction manager actually opens up a transaction. In such cases, prior to this change, the real exception is obfuscated, because the transaction manager would raise an exception about not being in a valid transactional state. With this change, attempting to abort a transaction that has not actually been opened will no longer result in an exception. Instead it will log a warning message and leave the transaction manager in its ready state.
1 parent d282a35 commit 2915dce

File tree

2 files changed

+10
-7
lines changed

2 files changed

+10
-7
lines changed

lib/kafka/transaction_manager.rb

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,8 @@ def abort_transaction
192192
end
193193

194194
unless @transaction_state.in_transaction?
195-
raise Kafka::InvalidTxnStateError, 'Transaction is not valid to abort'
195+
@logger.warn('Aborting transaction that was never opened on brokers')
196+
return
196197
end
197198

198199
@transaction_state.transition_to!(TransactionStateMachine::ABORTING_TRANSACTION)
@@ -250,6 +251,10 @@ def error?
250251
@transaction_state.error?
251252
end
252253

254+
def ready?
255+
@transaction_state.ready?
256+
end
257+
253258
def close
254259
if in_transaction?
255260
@logger.warn("Aborting pending transaction ...")

spec/transaction_manager_spec.rb

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -401,17 +401,15 @@
401401
manager.init_transactions
402402
end
403403

404-
it 'raises exception' do
405-
expect do
406-
manager.abort_transaction
407-
end.to raise_error(Kafka::InvalidTxnStateError, /transaction is not valid to abort/i)
404+
it 'does not raise an exception' do
405+
expect { manager.abort_transaction }.not_to raise_error
408406
end
409407

410-
it 'changes state to error' do
408+
it 'leaves transaction manager in ready state' do
411409
begin
412410
manager.abort_transaction
413411
rescue; end
414-
expect(manager.error?).to eql(true)
412+
expect(manager.ready?).to eql(true)
415413
end
416414
end
417415

0 commit comments

Comments
 (0)