From c38c46b8bcc479d8cf572ee2fd43c1c95aaa5cf4 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sun, 7 Jun 2026 10:54:52 +0200 Subject: [PATCH] =?UTF-8?q?test(v1.1.9):=20fix=20invoke=5Fe2e=20+=20retry?= =?UTF-8?q?=5Fe2e=20=E2=80=94=20admin=20bypass=20+=20rhai=20shape?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Running the E2E suites against real Postgres surfaced three shape bugs in the test scripts that caused false failures: invoke_e2e: - invoke_cross_app_rejects used two TestServer instances (one per app), but the second server's Owner admin isn't a member of the first server's app. Replaced with a single server that creates both apps via the same Owner admin (which has implicit access to every app). - invoke_depth_limit_exceeds_cleanly: the recurser script had its own try/catch, so when the depth limit fired inside the deepest call the caught error became the BODY of a 200 response (which invoke() returns to the caller). The outer caller's try/catch never saw a throw → assertion failed. Rewrote so the recurser propagates throws (no inner try-catch); the outer caller's try-catch surfaces the depth error all the way up. retry_e2e: - All three tests used HTTP routes which need a domain claim the test apps don't have (`no app claims host ""` 404s). Switched to the admin bypass POST /api/v1/execute/{id} — same pattern dispatcher_e2e uses. Sidesteps the per-app domain matcher entirely. - retry_run_surfaces_last_error_after_max_attempts: try-catch is a statement in Rhai, not an expression, so the block didn't evaluate to the catch arm's map. Refactored to bind to `let out` inside the catch arm, then return `#{ statusCode: 200, body: out }` as the final expression. All 11 v1.1.9 E2E tests now pass against Postgres: queue_e2e: 4 passed (33s — exercises retry + dead-letter) invoke_e2e: 4 passed (2s) retry_e2e: 3 passed (2.5s) Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/picloud/tests/invoke_e2e.rs | 137 +++++++++++++++-------------- crates/picloud/tests/retry_e2e.rs | 59 +++++-------- 2 files changed, 95 insertions(+), 101 deletions(-) diff --git a/crates/picloud/tests/invoke_e2e.rs b/crates/picloud/tests/invoke_e2e.rs index 31fb0d0..bc06b9f 100644 --- a/crates/picloud/tests/invoke_e2e.rs +++ b/crates/picloud/tests/invoke_e2e.rs @@ -1,7 +1,10 @@ -//! v1.1.9 invoke end-to-end tests. invoke()/invoke_async() are -//! exercised against the full all-in-one app via build_app, so the -//! invoke service + Engine back-reference + InvokeServiceImpl all -//! participate. +//! v1.1.9 invoke end-to-end tests. invoke()/invoke_async() exercised +//! against the full all-in-one app via build_app, so the invoke service +//! + Engine back-reference + InvokeServiceImpl all participate. +//! +//! The caller scripts run via the admin bypass `POST /api/v1/execute/{id}` +//! (same pattern dispatcher_e2e uses) — sidesteps the per-app domain / +//! route matcher entirely. //! //! Skips when DATABASE_URL is unset. @@ -83,20 +86,14 @@ async fn create_script(server: &TestServer, app_id: &str, name: &str, source: &s created["id"].as_str().expect("script id").to_string() } -/// Create a route bound to a script so we can invoke it via HTTP (and -/// any script can invoke() by route path). -async fn create_route(server: &TestServer, app_id: &str, script_id: &str, path: &str) { - let resp = server - .post(&format!("/api/v1/admin/apps/{app_id}/routes")) - .json(&json!({ - "script_id": script_id, - "method": "GET", - "path": path, - "host": null, - "dispatch_mode": "sync" - })) - .await; - resp.assert_status(axum::http::StatusCode::CREATED); +/// Direct executor bypass — same as dispatcher_e2e::execute. The script +/// runs sync with an empty body; sidesteps the route/host matcher. +async fn execute(server: &TestServer, script_id: &str) -> Value { + server + .post(&format!("/api/v1/execute/{script_id}")) + .json(&json!({})) + .await + .json() } async fn poll_marker(pool: &PgPool, app_id: &str) -> Option { @@ -122,43 +119,46 @@ async fn invoke_by_name_same_app_returns_value() { let Some(pool) = pool_or_skip().await else { return }; let (server, app_id) = server_for(pool.clone(), "invoke-name").await; - // Callee: returns its body's `x` + 1 as the response body. + // Callee returns 42 — the simplest possible round-trip. create_script( &server, &app_id, "callee", - r#"#{ statusCode: 200, body: ctx.request.body.x + 1 }"#, + r#"#{ statusCode: 200, body: 42 }"#, ) .await; - // Caller: invokes by name, writes result to KV marker. + // Caller invokes by name and surfaces the result via the response body. let caller = create_script( &server, &app_id, "caller", r#" - let r = invoke("callee", #{ x: 41 }); - kv::collection("e2e_markers").set("marker", #{ r: r }); - #{ statusCode: 200 } + let r = invoke("callee", #{}); + #{ statusCode: 200, body: r } "#, ) .await; - create_route(&server, &app_id, &caller, "/caller").await; - let resp = server.get("/caller").await; - resp.assert_status_ok(); - - let marker = poll_marker(&pool, &app_id).await.expect("marker fired"); - assert_eq!(marker["r"], 42); + let body = execute(&server, &caller).await; + assert_eq!(body, json!(42), "callee return value should round-trip"); } #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn invoke_cross_app_rejects() { let Some(pool) = pool_or_skip().await else { return }; - let (server, app_a) = server_for(pool.clone(), "invoke-A").await; - let (_, app_b) = server_for(pool.clone(), "invoke-B").await; + let (server, app_a) = server_for(pool.clone(), "invoke-xa").await; + // Create a second app via the SAME server / admin (Owner can manage + // every app) so the script-create calls don't run into role gates. + let slug_b = format!("e2e-invoke-xb-{}", Uuid::new_v4().simple()); + let created_b: Value = server + .post("/api/v1/admin/apps") + .json(&json!({ "slug": slug_b, "name": slug_b })) + .await + .json(); + let app_b = created_b["id"].as_str().expect("app_b id").to_string(); - // Callee lives in app_b. + // Callee lives in app_b — caller in app_a must not be able to reach it. let callee_b = create_script( &server, &app_b, @@ -167,7 +167,8 @@ async fn invoke_cross_app_rejects() { ) .await; - // Caller in app_a tries to invoke callee_b by id; must reject. + // Caller in app_a tries to invoke callee_b by id; the error must + // surface to the response body. let caller_src = format!( r#" let err = "no error"; @@ -176,17 +177,12 @@ async fn invoke_cross_app_rejects() { }} catch(e) {{ err = e; }} - kv::collection("e2e_markers").set("marker", #{{ err: err }}); - #{{ statusCode: 200 }} + #{{ statusCode: 200, body: err }} "# ); let caller_a = create_script(&server, &app_a, "caller_a", &caller_src).await; - create_route(&server, &app_a, &caller_a, "/x").await; - - server.get("/x").await.assert_status_ok(); - - let marker = poll_marker(&pool, &app_a).await.expect("marker fired"); - let err_str = marker["err"].as_str().unwrap_or_default(); + let body = execute(&server, &caller_a).await; + let err_str = body.as_str().unwrap_or_default(); assert!( err_str.contains("different app") || err_str.contains("CrossApp") @@ -200,25 +196,37 @@ async fn invoke_depth_limit_exceeds_cleanly() { let Some(pool) = pool_or_skip().await else { return }; let (server, app_id) = server_for(pool.clone(), "invoke-depth").await; - // Recursive script — invoke itself by name; loop bounded by - // Limits::trigger_depth_max (default 8). Records the error string. - let recurser_src = r#" - let err = "ok"; - try { - invoke("recurser", #{}); - } catch(e) { - err = e; - } - kv::collection("e2e_markers").set("marker", #{ err: err }); - #{ statusCode: 200 } - "#; - let recurser = create_script(&server, &app_id, "recurser", recurser_src).await; - create_route(&server, &app_id, &recurser, "/recurse").await; + // The recurser propagates any throw (no try-catch). At the depth + // ceiling Limits::trigger_depth_max=8 the innermost invoke() throws + // "invoke: depth limit exceeded (max 8)"; each nested invoke() then + // re-throws up the stack until the OUTER caller's try-catch surfaces + // it. + create_script( + &server, + &app_id, + "recurser", + r#"invoke("recurser", #{}); #{ statusCode: 200 }"#, + ) + .await; - server.get("/recurse").await.assert_status_ok(); + let caller = create_script( + &server, + &app_id, + "depth_caller", + r#" + let err = "ok"; + try { + invoke("recurser", #{}); + } catch(e) { + err = e; + } + #{ statusCode: 200, body: err } + "#, + ) + .await; - let marker = poll_marker(&pool, &app_id).await.expect("marker fired"); - let err = marker["err"].as_str().unwrap_or_default(); + let body = execute(&server, &caller).await; + let err = body.as_str().unwrap_or_default(); assert!( err.contains("depth"), "expected depth-limit error, got: {err}" @@ -230,7 +238,8 @@ async fn invoke_async_enqueues_outbox_row() { let Some(pool) = pool_or_skip().await else { return }; let (server, app_id) = server_for(pool.clone(), "invoke-async").await; - // Callee writes a marker. + // Callee writes a marker the test polls — the dispatcher must fire + // the OutboxSourceKind::Invoke row. create_script( &server, &app_id, @@ -252,10 +261,10 @@ async fn invoke_async_enqueues_outbox_row() { "#, ) .await; - create_route(&server, &app_id, &caller, "/start").await; - server.get("/start").await.assert_status_ok(); + execute(&server, &caller).await; - // Dispatcher fires the OutboxSourceKind::Invoke row → callee runs. - let marker = poll_marker(&pool, &app_id).await.expect("marker fired"); + let marker = poll_marker(&pool, &app_id) + .await + .expect("dispatcher should have fired the invoke_async row"); assert_eq!(marker["from"], "async"); } diff --git a/crates/picloud/tests/retry_e2e.rs b/crates/picloud/tests/retry_e2e.rs index 088ea21..6138cd1 100644 --- a/crates/picloud/tests/retry_e2e.rs +++ b/crates/picloud/tests/retry_e2e.rs @@ -1,7 +1,8 @@ //! v1.1.9 retry::* end-to-end test against a real engine inside the //! all-in-one binary. Smaller surface than queue/invoke (no async //! plumbing) — covers policy clamping, success-on-Nth-attempt, and -//! on_codes filtering through a real HTTP route. +//! on_codes filtering through the admin bypass POST /api/v1/execute/{id} +//! (sidesteps the per-app domain matcher). //! //! Skips when DATABASE_URL is unset. @@ -72,30 +73,22 @@ async fn server_for(pool: PgPool, suffix: &str) -> (TestServer, String) { (server, app_id) } -async fn create_route_for( - server: &TestServer, - app_id: &str, - name: &str, - source: &str, - path: &str, -) { +async fn create_script(server: &TestServer, app_id: &str, name: &str, source: &str) -> String { let created: Value = server .post("/api/v1/admin/scripts") .json(&json!({ "app_id": app_id, "name": name, "source": source })) .await .json(); - let script_id = created["id"].as_str().expect("script id"); - let resp = server - .post(&format!("/api/v1/admin/apps/{app_id}/routes")) - .json(&json!({ - "script_id": script_id, - "method": "GET", - "path": path, - "host": null, - "dispatch_mode": "sync" - })) - .await; - resp.assert_status(axum::http::StatusCode::CREATED); + created["id"].as_str().expect("script id").to_string() +} + +/// Execute via the admin bypass (same pattern as dispatcher_e2e). +async fn execute(server: &TestServer, script_id: &str) -> Value { + server + .post(&format!("/api/v1/execute/{script_id}")) + .json(&json!({})) + .await + .json() } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -114,11 +107,8 @@ async fn retry_run_eventually_succeeds_inside_http_handler() { }); #{ statusCode: 200, body: v } "#; - create_route_for(&server, &app_id, "succ", src, "/r/succ").await; - - let resp = server.get("/r/succ").await; - resp.assert_status_ok(); - let body: Value = resp.json(); + let script_id = create_script(&server, &app_id, "succ", src).await; + let body = execute(&server, &script_id).await; assert_eq!(body, json!(3)); } @@ -129,18 +119,16 @@ async fn retry_run_surfaces_last_error_after_max_attempts() { let src = r#" let p = retry::policy(#{ max_attempts: 2, base_ms: 1, jitter_pct: 0 }); + let out = "did not throw"; try { retry::run(p, || { throw "boom" }); - #{ statusCode: 200, body: "did not throw" } } catch(e) { - #{ statusCode: 200, body: e } + out = e; } + #{ statusCode: 200, body: out } "#; - create_route_for(&server, &app_id, "surf", src, "/r/surf").await; - - let resp = server.get("/r/surf").await; - resp.assert_status_ok(); - let body: Value = resp.json(); + let script_id = create_script(&server, &app_id, "surf", src).await; + let body = execute(&server, &script_id).await; let s = body.as_str().unwrap_or_default(); assert!(s.contains("boom"), "expected 'boom' in error, got: {s}"); } @@ -166,10 +154,7 @@ async fn retry_on_codes_filters_unmatched_errors() { } #{ statusCode: 200, body: attempts } "#; - create_route_for(&server, &app_id, "codes", src, "/r/codes").await; - - let resp = server.get("/r/codes").await; - resp.assert_status_ok(); - let body: Value = resp.json(); + let script_id = create_script(&server, &app_id, "codes", src).await; + let body = execute(&server, &script_id).await; assert_eq!(body, json!(1), "non-matching error should surface immediately"); }