Why does the drop of a connection panic?

[GunnarMorrigan]

Hi,

I dropped a connection which had produced an error which in turn panicked while dropping it.
Why does the drop implementation panic? Is an error message not a better approach?

odbc-api/odbc-api/src/connection.rs

Lines 33 to 52 in 55bba0f

	panic!(
	"Unexpected error rolling back transaction (In order to recover from \
	invalid transaction state during disconnect): {e:?}"
	)
	}
	}
	// Transaction is rolled back. Now let's try again to disconnect.
	if let Err(e) = self.connection.disconnect().into_result(&self.connection) {
	// Avoid panicking, if we already have a panic. We don't want to mask the original
	// error.
	if !panicking() {
	panic!("Unexpected error disconnecting): {e:?}")
	}
	}
	}
	Err(e) => {
	// Avoid panicking, if we already have a panic. We don't want to mask the original
	// error.
	if !panicking() {
	panic!("Unexpected error disconnecting: {e:?}")

[pacman82]

Hello @GunnarMorrigan ,

my personal take on error handling in a nutshell:

• bug => panic
• runtime error => Error message to the operator and retry
• logic error => Error message to the user

So far if closing a connection failed it was due to programming errors. For me a panic is fine in these situations. Yet it should actually be really had to make such errors using the odbc-api bindings.

So I would be very interested in the nature and circumstances of the error. Either there is a way to prevent it in the programming model, or if it is indeed a runtime error, I could think about adding an explicit disconnect method allowing to return an error to handle for the user. I could see potential difficulties, if the error causes the environment to be invalid, or the connection still exists after, but all the more important to have more context.

Best, Markus

[GunnarMorrigan]

Hi @pacman82,

Thanks for the time and library.

I could agree with your mapping of problems. But i am not sure if this would be a bug or bad usage causing a bug.

{"timestamp":"2024-07-11T11:00:21.165267Z","level":"ERROR","fields":{"message":"Error storing entry: ODBC emitted an error calling 'SQLExecDirect':\nState: 08S01, Native error: 32, Message: [Microsoft][ODBC Driver 18 for SQL Server]TCP Provider: Error code 0x20"},"target":"storage_task","line_number":99}
thread 'db_storage_task' panicked at /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/odbc-api-5.1.0/src/connection.rs:27:25:
Unexpected error rolling back transaction (In order to recover from invalid transaction state during disconnect): Diagnostics { record: State: 08S01, Native error: 0, Message: [Microsoft][ODBC Driver 18 for SQL Server]Communication link failure, function: "SQLEndTran" }

I am not sure what TCP error 0x20 is but, to me this seems like a TCP failure that turns into a panic because the rollback is attempted on drop and errors as there is no connection. A panic in this situation is undesirable to me.

Maybe you have more insights to what might cause this and how to prevent it.

[pacman82]

Any chance you could gift me a piece of reproducing code?

[GunnarMorrigan]

I will to get this next week sure but cannot guarantee it will be reproducing code.

The environment in which it runs might have an impact on it. Also the run duration might cause the issue. I create a connection for the entirety of the program. Which is supposed to run for weeks.

[pacman82]

Fair, but maybe you could describe your use case a bit? I am trying to gauge, why the transaction is not terminated, even if it timed out.

[GunnarMorrigan]

Hi,

I have uploaded the database code that I am using. The storage task remains the most unchanged. The database helper struct has changed a little to remove some of the other two insert methods.
You can see that here as i commented out two other insert methods.

The crashing code is here:
https://github.com/GunnarMorrigan/odbc-code/blob/d2abd00951b35361a88a593c3431d37a030a0216/src/storage_task.rs#L61-L76

I have several things to note about my code / env I experienced these crashes in.

• I was / am using odbc-api v5 (just switched to the latest v8.1.2)
• Auto commit was turned off. The db commits and rollbacks should are handled in the storage task struct.
  This is mainly because I also upload a large blob to cloud storage. This requires me to only commit to DB after the blob is in the cloud to prevent orphans.

[pacman82]

Note to self: Connection might be in suspended state as documented here https://learn.microsoft.com/en-us/sql/odbc/reference/syntax/sqlendtran-function.

Attempting to disconnect without panic if we can verify connection is in suspended state, even if rollback failed.

[pacman82]

Hello @GunnarMorrigan,

odbc-api 8.1.4 has just been released. The rollback will no longer panic. The assumption is that in your situation (i.e. a suspended transaction), the second call to disconnect will succeed and therefore the drop handler over all will not panic and free all associated resources.

I can only vouch for really that it won't be the rollback that panics now. Would you mind giving odbc-api 8.1.4 a spin and see if it solves your issue, or just moves on to a different panic?

Best, Markus

[pacman82]

Really hard to reproduce in an automated integration test. Yet I could reproduce the bug manually, by shutting down the database in just the right moment.

The newest version odbc-api 8.1.4 fixes this. Closing this issue.