Automigration: improve error handling (clearer communication with the caller)
TL;DR
The errors occurring in automigration function(s) aren't always communicated clearly to the caller. I see two points to this issue:
- make sure we're using the correct DB interface (
local_db/mem_db) when retrieving the error (db.errmsg()) and communicating it back to the caller - make sure errors in
local_dbcontext live long enough to be communicated back to the caller (I gotOKwithnot an errorwhen I knew for a fact --through alternative logging/tracing methods-- that an error occurred inlocal_dbctx)
The Issue
If an error happens during crsql_automigrate execution (and is propagated up using try shorthand ?), it is always handled as if actions on mem_db have caused the error:
// automigrate.rs:automigrate_impl
// It's possible that migrate_result = Err(rc) originating from local_db or mem_db
if let Err(_) = migrate_result {
local_db.exec_safe("ROLLBACK")?;
// The error gets communicated as if we know for certain
// that it originated from mem_db
let mem_db_err_msg = mem_db.errmsg()?;
ctx.result_error(&mem_db_err_msg);
ctx.result_error_code(mem_db.errcode());
cleanup(mem_db)?;
return Err(ResultCode::OK);
}
I think this should be handled properly: by letting the wrapping code if let Err(_) = migrate_result { ... } know the source of the error so that it can pull errmsg() using the correct DB interface and communicate the error to the caller, something like:
// automigrate.rs:automigrate_imp
if let Err(err) = migrate_result {
local_db.exec_safe("ROLLBACK")?;
let errsrc = figure_out_error_source(err, local_db, mem_db);
let errcode = errsrc.errcode();
let errmsg = errsrc.errmsg()?;
ctx.result_error(&errmsg);
ctx.result_error_code(errcode);
cleanup(mem_db)?;
return Err(ResultCode::OK);
}
Furthermore, errors having happened in local_db context are cleared before the execution is done: I've tried fixing the previous point and, even though migrate_result returned an error that an error occurred in local_db (and I knew for a fact -- through alternative logging/tracing), local_db.errcode() and local_db.errmsg() returned OK and not an error (respectively). This, I believe, has something to do with calling context (how deep into the call we are) but am still investigating.
The Symptom
If using a JS wrapper with wa-sqlite, wa-sqlite validates the return code and throws an error if != 0 (for intents and purposes), so in this case:
- error happens in
local_db - the
if Err(_) ...block retrievesmem_dberror code/message (which areOK) and communicates back to caller - wa-sqlite's validation sees 0 and treats this as a non error