My question concerns catching errors and rolling back database transactions when the transaction spans a few levels of procedures calls.
I wrote the code, first, just to get it working and used a lot of if [catch {db eval {...}} result catchDict] statements for all the database code; and then attempted to wrap it in a transaction.
It became necessary to re-throw the errors that were caught and I tried code like
if [catch {db eval {...}} result catchDict] {
dict incr catchDict -level 2
return -options $catchDict $result
}
which appeared to work in terms of propagating the errors up to the calling procedure. However, it did not work with db transaction {...} but I may have done something wrong, of course. I tried
if [catch {db transaction immediate {::DOCS::AppendToUndoChain $request}} result catchDict] {
puts "rolling back"
puts $result
puts $catchDict
}
which stopped execution in AppendToUndoChain at the point of error but did not roll back database changes performed within procedures called/invoked by AppendToUndoChain or a procedure under that.
The following seems to work fine and rolls back everything regardless of which level of procedure call the query was performed.
db eval {begin immediate;}
if [catch {::DOCS::AppendToUndoChain $request} result catchDict] {
db eval {rollback;}
puts "Rolled back."
puts $result
} else {
db eval {commit;}
}
Question 1: Am I doing something wrong with the db transaction {...} or the re-throwing of errors such that it won't roll back across all procedures?
Question 2: Is a catch on every db eval {...} really needed or can the one around the call to ::DOCS::AppendToUndoChain take care of it all, such that all errors in that procedure and any procedure "below" it will propagate "up" to this catch and handle the errors and roll back of the transaction, such that all the queries can be coded as set result [db eval {...}] rather than if [catch {db eval {...}} result]?
I've tried introducing different types of errors in procedures at different levels relative to this single/outer catch and it appears to catch them all and roll back the complete transaction; but I'd like to know if this is a sound approach and what I might be overlooking.
Thank you for considering my question.
Catching and rethrowing exceptions transparently is a little more subtle than you might expect (or than the documentation makes clear): the cases break / continue / return; error; and ok are all a little different in terms of the fiddles needed to rethrow them transparently:
proc my_transaction script {
set rollback 0
# start transaction
try {
uplevel 1 $script
} on return {r o} - on break {r o} - on continue {r o} {
dict incr o -level 1
return -options $o $r
} on error {r o} {
set rollback 1
return -options $o $r
} on ok r {
return $r
} finally {
if {$rollback} {
# do rollback
} else {
# do commit
}
}
}
This is what (in effect) sqlite is doing on the c level with its db transaction {...} command, with the small addition of keeping track of the transaction nesting and calling ROLLBACK TO _tcl_transaction ; RELEASE _tcl_transaction or RELEASE _tcl_transaction if nested instead of ROLLBACK and COMMIT respectively. But if you want to do similar (that is: run some code in a section and do some non-db commit / rollback actions based on its exception status, then rethrow the exception so that the outer db transaction does the right thing, then you'll need to use a similar construction.
How that looks will depend on your use case (it's not quite clear from your question). If you're doing a bunch of things that must happen atomically on the db level, and just report an error if something fails then the idiom would be something like:
try {
db transaction {
foreach client $clients {
set name [dict get $client name]
db eval {insert into clients (name) values (:name)}
foreach addr [dict get $client addresses] {
db eval {insert into addresses (client, addr) values (:client, :addr)}
}
}
}
puts "will only be reached if no exception was thrown"
} on error {errmsg options} {
puts stderr "Error doing things: [dict get $options -errorinfo]"
} on ok {} {
puts "Things went fine"
}
Nice and simple: either all the client addresses are loaded, or none are, if the code loading them throws an error (exactly code TCL_ERROR - anything else: TCL_BREAK, TCL_CONTINUE, etc are all treated as success by sqlite's db transaction and will trigger a commit).
But if the code within the db transaction needs to do its own exception handling with rethrowing of exceptions you'll need something like:
proc with_logo {logovar client script} {
upvar 1 $logovar logo
set old [pwd]
set tmpdir [file tempdir]
try {
cd $tmpdir
set logo client_logo[expr {int(rand()*1000)}].png
exec curl -o $logo [dict get $client logo_url]
uplevel 1 $script
} on break {r o} - on continue {r o} {
dict incr o -level 1
return -options $o $r
} on return {r o} {
dict incr o -level 1
dict set o -code return
return -options $o $r
} finally {
cd $old
file delete -force $tmpdir
}
}
try {
db transaction {
foreach client $clients {
with_logo fn $client {
set logo_md5 [exec md5sum $fn]
set logo_size [file size $fn]
if {$logo_size == 0} {
puts "client [dict get $client name]'s logo is empty"
continue ;# skip clients with broken logos
}
}
set name [dict get $client name]
db eval {insert into clients (name, logo_md5, logo_size) values (:name, :logo_md5, :logo_size)}
foreach addr [dict get $client addresses] {
db eval {insert into addresses (client, addr) values (:client, :addr)}
}
}
}
puts "will only be reached if no exception was thrown"
} on error {errmsg options} {
puts stderr "Error doing things: [dict get $options -errorinfo]"
} on ok {} {
puts "Things went fine"
}
Just an aside: be careful of code like: if [catch {foo}] {...}: the first arg of if is an expression. Written like this it will take the result of the catch command and reparse it as a Tcl expression (a different language) with another round of substitutions, which is difficult to reason about and a rich source of remote code execution security vulnerabilities. For this reason I strongly advise always brace-quoting expressions (like expr {$a + $b} rather than expr $a + $b, or, like this case, the expression args of if, while, for, etc. In this exact case it's safe, but for fragile reasons: the return value of catch will always be an integer, whose string representation can never be interpreted as something that will trigger additional dangerous substitutions during the expr parse round.