diff --git a/CHANGELOG.md b/CHANGELOG.md index 916f309..1d8b2df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Add support for Bolt 5.2, which adds notification filtering. - Add `Driver::is_encrypted()`. - Reduce the number of lifetime generic parameters in `TransactionQueryBuilder` and `TransactionRecordStream`. + - Fix `Transaction::rolblack()` failing if a result stream failed before. ## 0.0.2 - Update dependencies. diff --git a/neo4j/src/driver/transaction.rs b/neo4j/src/driver/transaction.rs index 7d9f826..d3f23dc 100644 --- a/neo4j/src/driver/transaction.rs +++ b/neo4j/src/driver/transaction.rs @@ -86,14 +86,26 @@ impl<'driver, 'tx> Transaction<'driver, 'tx> { /// This is the default behavior when the transaction is dropped. /// However, when dropping the transaction, potential errors will be swallowed. pub fn rollback(self) -> Result<()> { - self.drop_result.into_inner()?; - self.inner_tx.rollback() + match self.drop_result.into_inner() { + Ok(_) => self.inner_tx.rollback(), + Err(_) => { + // Nothing to do here. + // The transaction already failed and doesn't need to be rolled back. + Ok(()) + } + } } } /// A result cursor as returned by [`TransactionQueryBuilder::run()`]. /// /// It implements [`Iterator`] and can be used to iterate over the [`Record`]s. +/// +/// Before ending the transaction ([`Transaction::commit()`] or [`Transaction::rollback()`]), all +/// record streams spawned from it must be dropped. +/// While calling [`drop(stream)`] works fine for this purpose, it will swallow any outstanding +/// errors. +/// Therefore, it is recommended to use [`TransactionRecordStream::consume()`] instead. #[derive(Debug)] pub struct TransactionRecordStream<'driver, 'tx>( RecordStream<'driver>, diff --git a/testkit_backend/src/testkit_backend/responses.rs b/testkit_backend/src/testkit_backend/responses.rs index e8403d7..eb40fd1 100644 --- a/testkit_backend/src/testkit_backend/responses.rs +++ b/testkit_backend/src/testkit_backend/responses.rs @@ -112,12 +112,9 @@ static REGEX_SKIPPED_TESTS: OnceLock> = Once fn get_plain_skipped_tests() -> &'static HashMap<&'static str, &'static str> { PLAIN_SKIPPED_TESTS.get_or_init(|| { - let mut map = HashMap::new(); - map.insert( - "neo4j.test_tx_run.TestTxRun.test_tx_res_fails_whole_tx", - "backend (SessionHolder) doesn't keep result buffers around after error", - ); - map + HashMap::from([ + // ("path.to.skipped_test", "reason"), + ]) }) } diff --git a/testkit_backend/src/testkit_backend/session_holder.rs b/testkit_backend/src/testkit_backend/session_holder.rs index 417291c..42df3da 100644 --- a/testkit_backend/src/testkit_backend/session_holder.rs +++ b/testkit_backend/src/testkit_backend/session_holder.rs @@ -470,15 +470,10 @@ impl SessionHolderRunner { } Err(err) => { known_transactions.insert(id, TxFailState::Failed); - // outside the receiver, we'll just reply with an - // error to the command. So query and parameters - // don't matter. - _ = buffered_command.insert(TransactionRun { - transaction_id: command.transaction_id, - query: String::from(""), - params: None, - }.into()); - break Err(err); + let msg = TransactionRunResult { + result: Err(err.into()), + }; + tx_res.send(msg.into()).unwrap(); } } }