Surface execution-history lookup failures in df.instance_executions (#168) (#225)

df.instance_executions() previously swallowed every failure (runtime build, provider connect, list_executions, get_execution_info) into an empty rowset. Because a completed instance always has at least one execution row, an empty result was indistinguishable from a silently failed lookup, masking real errors (issue #168).

These failures now raise an explicit error, while the genuinely-empty case (a non-existent or non-owned instance, gated by the RLS existence check) still returns an empty rowset. Also validate limit_count >= 1 and cap it at 10000, matching df.list_instances.

Add e2e regression test tests/e2e/sql/05_instance_executions.sql.

Author: crprashant

src/monitoring.rs

@@ -192,6 +192,14 @@ pub fn instance_info(
 }
 
 /// Get the last N executions for an eternal durable function (loop).
+///
+/// Distinguishes "this instance genuinely has no execution history yet" (empty
+/// rowset) from "the execution-history lookup failed" (explicit error). The
+/// latter — failing to build the runtime, connect to the duroxide store, list
+/// executions, or fetch a specific execution's info — now raises an error
+/// instead of being silently swallowed into an empty rowset. A completed
+/// instance always has at least one execution row, so an empty result for one
+/// previously masked a real lookup failure. See issue #168.
 #[pg_extern(schema = "df")]
 pub fn instance_executions(
     instance_id: &str,
@@ -206,11 +214,18 @@ pub fn instance_executions(
         name!(output, Option<String>),
     ),
 > {
+    if limit_count < 1 {
+        pgrx::error!("limit_count must be at least 1");
+    }
+    let limit_count = limit_count.min(10000);
+
     let pg_conn_str = postgres_connection_string();
     let provider_schema = backend_duroxide_schema();
     let instance_id_owned = instance_id.to_string();
 
     // Ownership check: SPI goes through RLS, so non-owned instances are invisible.
+    // A non-existent or non-owned instance legitimately has no history to show,
+    // so an empty rowset (not an error) is the correct response here.
     let exists: bool = Spi::get_one_with_args(
         "SELECT EXISTS(SELECT 1 FROM df.instances WHERE id = $1)",
         &[instance_id.into()],
@@ -228,29 +243,31 @@ pub fn instance_executions(
         .build()
     {
         Ok(rt) => rt,
-        Err(_) => return TableIterator::new(vec![]),
+        Err(e) => pgrx::error!("failed to create async runtime for instance_executions: {e}"),
     };
 
-    let results = rt.block_on(async {
-        let store = match new_backend_provider(&pg_conn_str, provider_schema).await {
-            Ok(s) => s,
-            Err(_) => return vec![],
-        };
+    let results: Result<Vec<(i64, String, i64, i64, Option<String>)>, String> =
+        rt.block_on(async {
+            let store = new_backend_provider(&pg_conn_str, provider_schema).await?;
 
-        let client = Client::new(store);
+            let client = Client::new(store);
 
-        let execution_ids = match client.list_executions(&instance_id_owned).await {
-            Ok(ids) => ids,
-            Err(_) => return vec![],
-        };
+            let execution_ids = client
+                .list_executions(&instance_id_owned)
+                .await
+                .map_err(|e| format!("failed to list executions: {e:?}"))?;
 
-        let mut sorted_ids: Vec<_> = execution_ids.into_iter().collect();
-        sorted_ids.sort_by(|a, b| b.cmp(a));
-        let limited: Vec<_> = sorted_ids.into_iter().take(limit_count as usize).collect();
+            let mut sorted_ids: Vec<_> = execution_ids.into_iter().collect();
+            sorted_ids.sort_by(|a, b| b.cmp(a));
+            let limited: Vec<_> = sorted_ids.into_iter().take(limit_count as usize).collect();
+
+            let mut rows = Vec::new();
+            for exec_id in limited {
+                let info = client
+                    .get_execution_info(&instance_id_owned, exec_id)
+                    .await
+                    .map_err(|e| format!("failed to fetch info for execution {exec_id}: {e:?}"))?;
 
-        let mut rows = Vec::new();
-        for exec_id in limited {
-            if let Ok(info) = client.get_execution_info(&instance_id_owned, exec_id).await {
                 let duration_ms = info
                     .completed_at
                     .map(|end| end.saturating_sub(info.started_at))
@@ -264,11 +281,13 @@ pub fn instance_executions(
                     info.output,
                 ));
             }
-        }
-        rows
-    });
+            Ok(rows)
+        });
 
-    TableIterator::new(results)
+    match results {
+        Ok(rows) => TableIterator::new(rows),
+        Err(e) => pgrx::error!("df.instance_executions: execution history lookup failed: {e}"),
+    }
 }
 
 /// Get system-wide durable function metrics.

tests/e2e/sql/05_instance_executions.sql

@@ -0,0 +1,94 @@
+-- Copyright (c) Microsoft Corporation.
+-- Licensed under the PostgreSQL License.
+
+-- Regression test for issue #168:
+--   df.instance_executions() can return no rows for completed instances.
+--
+-- A completed instance always has at least one execution row in the duroxide
+-- store (the instance and its first execution are created in the same
+-- transaction). Previously, instance_executions() swallowed any
+-- runtime/provider/lookup error into an empty rowset, making "no execution
+-- history yet" indistinguishable from "the lookup failed".
+--
+-- This test guards the contract:
+--   * a completed instance exposes >= 1 execution row, and
+--   * an invalid limit_count is rejected with an explicit error rather than
+--     being silently masked by an empty rowset.
+SET SESSION AUTHORIZATION df_e2e_user;
+
+CREATE TEMP TABLE _test_state (instance_id TEXT);
+
+INSERT INTO _test_state SELECT df.start('SELECT 42', 'test-instance-executions-168');
+
+DO $$
+DECLARE
+    inst_id    TEXT;
+    status     TEXT;
+    exec_count INT;
+    top_exec_id BIGINT;
+    top_status TEXT;
+BEGIN
+    SELECT instance_id INTO inst_id FROM _test_state;
+    RAISE NOTICE 'Testing instance: %', inst_id;
+
+    SELECT df.wait_for_completion(inst_id) INTO status;
+    IF lower(status) != 'completed' THEN
+        RAISE EXCEPTION 'TEST FAILED: expected completed, got %', status;
+    END IF;
+
+    -- Core assertion (issue #168): a completed instance must expose >= 1 execution.
+    SELECT count(*) INTO exec_count FROM df.instance_executions(inst_id, 1);
+    IF exec_count < 1 THEN
+        RAISE EXCEPTION 'TEST FAILED: instance_executions(inst_id, 1) returned % rows for a completed instance, expected >= 1', exec_count;
+    END IF;
+
+    -- The returned execution should have a valid id and a non-empty status.
+    -- We deliberately do NOT assert status = 'completed': df.status() /
+    -- wait_for_completion track the pg_durable df.instances table, while
+    -- instance_executions reports duroxide's per-execution status. The two are
+    -- updated independently and can briefly diverge under concurrent
+    -- background-worker load. Issue #168 is about empty rows, not the exact
+    -- status string, so asserting row presence + a well-formed status is the
+    -- correct, race-free check.
+    SELECT e.execution_id, e.status INTO top_exec_id, top_status
+    FROM df.instance_executions(inst_id, 1) e;
+    IF top_exec_id < 1 THEN
+        RAISE EXCEPTION 'TEST FAILED: execution_id should be >= 1, got %', top_exec_id;
+    END IF;
+    IF top_status IS NULL OR length(trim(top_status)) = 0 THEN
+        RAISE EXCEPTION 'TEST FAILED: execution status should be non-empty, got %', coalesce(top_status, '<null>');
+    END IF;
+
+    -- The default limit should also return the execution history.
+    SELECT count(*) INTO exec_count FROM df.instance_executions(inst_id);
+    IF exec_count < 1 THEN
+        RAISE EXCEPTION 'TEST FAILED: instance_executions(inst_id) (default limit) returned % rows, expected >= 1', exec_count;
+    END IF;
+
+    RAISE NOTICE 'TEST PASSED: instance_executions returns execution history for a completed instance';
+END $$;
+
+-- limit_count < 1 must raise an explicit error, not silently return empty rows.
+DO $$
+DECLARE
+    inst_id   TEXT;
+    got_error BOOLEAN := false;
+BEGIN
+    SELECT instance_id INTO inst_id FROM _test_state;
+    BEGIN
+        PERFORM * FROM df.instance_executions(inst_id, 0);
+    EXCEPTION WHEN OTHERS THEN
+        got_error := true;
+    END;
+
+    IF NOT got_error THEN
+        RAISE EXCEPTION 'TEST FAILED: instance_executions(inst_id, 0) should raise an error for limit_count < 1';
+    END IF;
+
+    RAISE NOTICE 'TEST PASSED: instance_executions rejects limit_count < 1';
+END $$;
+
+DROP TABLE _test_state;
+
+RESET SESSION AUTHORIZATION;
+SELECT 'TEST PASSED' AS result;