Check every statement in the webhook deletion transaction (closes #262)

deleteWebhookResources issued three deletes without checking any of
them. A failing delete sets .Error on the returned session but leaves
the transaction usable, so the handler committed whatever succeeded,
redirected to /sources as though deletion had worked, and then
hard-deleted the per-webhook event database anyway. A webhook could
end up with its entrypoints or targets still in the main database and
its entire event history permanently gone, reported as a success.

The transaction moves into commitWebhookDeletion, which checks each
statement and rolls back on any failure, matching commitWebhook in the
same file. deleteWebhookResources reports the failure with
h.serverError and leaves the event database alone.

The configuration commit deliberately precedes DeleteDB: no
transaction spans SQLite and the filesystem, and a failure after the
commit leaves an unreferenced event database file, which the operator
can remove, rather than destroying history for a webhook that still
exists. That failure is reported with h.serverError too instead of a
success redirect.

internal/handlers/source_management.go

@@ -625,43 +625,27 @@ func (h *Handlers) deleteWebhookResources(
 	webhook database.Webhook,
 	userID string,
 ) {
-	tx := h.db.DB().Begin()
-	if tx.Error != nil {
-		h.log.Error(
-			"failed to begin transaction",
-			"error", tx.Error,
-		)
-		http.Error(
-			w, "Internal server error",
-			http.StatusInternalServerError,
-		)
-
-		return
-	}
-
-	tx.Where(
-		"webhook_id = ?", webhook.ID,
-	).Delete(&database.Entrypoint{})
-
-	tx.Where(
-		"webhook_id = ?", webhook.ID,
-	).Delete(&database.Target{})
-
-	tx.Delete(&webhook)
-
-	err := tx.Commit().Error
+	// The configuration delete commits before the event database
+	// is touched. No transaction spans the main database and the
+	// filesystem, so one side has to go first: committing the
+	// configuration first means a later failure leaves an unused
+	// event database file on disk, while removing the event
+	// database first would mean a failed commit destroys the
+	// history of a webhook that still exists. A leftover file can
+	// be removed by hand; deleted history cannot be recovered.
+	err := h.commitWebhookDeletion(&webhook)
 	if err != nil {
-		h.log.Error(
-			"failed to commit deletion", "error", err,
-		)
-		http.Error(
-			w, "Internal server error",
-			http.StatusInternalServerError,
-		)
+		h.serverError(w, "failed to delete webhook", err)
 
 		return
 	}
 
+	h.log.Info(
+		"webhook deleted",
+		"webhook_id", webhook.ID,
+		"user_id", userID,
+	)
+
 	// Release the delivery engine's per-webhook archiving state
 	// so a deleted webhook's archive writer (and any handle open
 	// within its debounce window) does not linger for the
@@ -671,22 +655,63 @@ func (h *Handlers) deleteWebhookResources(
 
 	err = h.dbMgr.DeleteDB(webhook.ID)
 	if err != nil {
-		h.log.Error(
-			"failed to delete webhook event database",
-			"webhook_id", webhook.ID,
-			"error", err,
+		// The configuration is committed, so the webhook is gone,
+		// but its event database file is still on disk with
+		// nothing referencing it. Report the failure rather than
+		// redirecting as though everything succeeded: the file
+		// needs removing by hand, and the logged error names it.
+		h.serverError(
+			w, "failed to delete webhook event database", err,
 		)
+
+		return
 	}
 
-	h.log.Info(
-		"webhook deleted",
-		"webhook_id", webhook.ID,
-		"user_id", userID,
-	)
-
 	http.Redirect(w, r, "/sources", http.StatusSeeOther)
 }
 
+// commitWebhookDeletion soft-deletes a webhook's entrypoints,
+// targets and the webhook row in one transaction. Every
+// statement is checked and any failure rolls the whole
+// transaction back, so a caller that gets an error knows the
+// configuration is untouched and the event database must be
+// left alone.
+func (h *Handlers) commitWebhookDeletion(
+	webhook *database.Webhook,
+) error {
+	tx := h.db.DB().Begin()
+	if tx.Error != nil {
+		return tx.Error
+	}
+
+	err := tx.Where(
+		"webhook_id = ?", webhook.ID,
+	).Delete(&database.Entrypoint{}).Error
+	if err != nil {
+		tx.Rollback()
+
+		return err
+	}
+
+	err = tx.Where(
+		"webhook_id = ?", webhook.ID,
+	).Delete(&database.Target{}).Error
+	if err != nil {
+		tx.Rollback()
+
+		return err
+	}
+
+	err = tx.Delete(webhook).Error
+	if err != nil {
+		tx.Rollback()
+
+		return err
+	}
+
+	return tx.Commit().Error
+}
+
 // evictArchiveWriter asks the delivery engine to drop its
 // cached archive writer for a webhook, closing the archive file
 // handle.