From e22962bf59926da33e227c9305bf8d93303f719e Mon Sep 17 00:00:00 2001 From: Richard McDaniel <1130888+rmcdaniel@users.noreply.github.com> Date: Thu, 29 Oct 2026 04:33:36 +0000 Subject: [PATCH 1/4] Rust #325: answer worker-plane errors as published PHP does Sending the same malformed worker requests to published 2.11.5 and native showed four differences: - a workflow or activity poll failing its request rules was answered with native's own reasons, not Laravel's validation_failed in the worker envelope; - an unknown activity task on the fail and heartbeat routes was answered with a bare message and reason; - in a named namespace, a route PHP does not have was refused as an unqualified named-namespace operation instead of PHP's 404 or 405; - a completion naming an unknown workflow task was refused for its command payloads before PHP's 404 task_not_found. Validate both polls with the published rules, answer an unknown activity task with guardAttemptOwnership's body on every route, give PHP's 404 or 405 for unrouted requests in any namespace, and look the workflow task up before checking or fetching command payloads. The new worker-error-bodies fixture records ten such requests against published 2.11.5. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/server-parity.yml | 3 +- .../2026-10-29-worker-error-bodies.md | 48 +++ docs/rust-server-port.md | 5 - rust/src/runtime/execution.rs | 10 + rust/src/runtime/http.rs | 66 +++- rust/src/runtime/store.rs | 11 + rust/tests/execution.rs | 2 + rust/tests/execution/worker_error_bodies.rs | 99 ++++++ .../conformance/server-parity/contract.mjs | 3 + scripts/conformance/server-parity/probe.php | 6 + .../worker-error-bodies-contract.mjs | 24 ++ .../worker-error-bodies-probe.php | 38 +++ .../ServerParity/worker-error-bodies.json | 294 ++++++++++++++++++ ...verParityWorkerErrorBodiesContractTest.mjs | 40 +++ 14 files changed, 640 insertions(+), 9 deletions(-) create mode 100644 docs/rust-server-port-checkpoints/2026-10-29-worker-error-bodies.md create mode 100644 rust/tests/execution/worker_error_bodies.rs create mode 100644 scripts/conformance/server-parity/worker-error-bodies-contract.mjs create mode 100644 scripts/conformance/server-parity/worker-error-bodies-probe.php create mode 100644 tests/Fixtures/ServerParity/worker-error-bodies.json create mode 100644 tests/Unit/ServerParityWorkerErrorBodiesContractTest.mjs diff --git a/.github/workflows/server-parity.yml b/.github/workflows/server-parity.yml index 076be942a..842c1c9c2 100644 --- a/.github/workflows/server-parity.yml +++ b/.github/workflows/server-parity.yml @@ -63,6 +63,7 @@ on: - tests/Unit/ServerParityContinueTransfersContractTest.mjs - tests/Unit/ServerParityLargeCommandBatchesContractTest.mjs - tests/Unit/ServerParityManyUpdatesContractTest.mjs + - tests/Unit/ServerParityWorkerErrorBodiesContractTest.mjs - tests/Unit/ServerParityStartSearchAttributesContractTest.mjs - tests/Unit/ServerParityQueryArgumentsContractTest.mjs - tests/Unit/ServerParityQueryContractArgumentsContractTest.mjs @@ -1989,7 +1990,7 @@ jobs: bash scripts/ci/run-composer.sh --rm -v "$PWD:/source" -w /source -- \ install --working-dir=scripts/conformance/server-parity --no-interaction --no-progress docker run --rm --user=1000:1000 --entrypoint node -v "$PWD:/source:ro" -w /source \ - "$image" --test tests/Unit/ServerParityRunnerTest.mjs tests/Unit/ServerParityRunnerBlockedExcerptTest.mjs tests/Unit/ServerParityProbeNotesTest.mjs tests/Unit/ServerParitySdkDirectionLanesTest.mjs tests/Unit/ServerParityCodecDeadlineCellsTest.mjs tests/Unit/ServerParityCooperativeContractTest.mjs tests/Unit/ServerParityChildCancellationContractTest.mjs tests/Unit/ServerParityScheduleContractTest.mjs tests/Unit/ServerParityVisibilityContractTest.mjs tests/Unit/ServerParityAdmissionContractTest.mjs tests/Unit/ServerParityRoleTokenContractTest.mjs tests/Unit/ServerParityWorkerDeregistrationContractTest.mjs tests/Unit/ServerParitySearchAttributeContractTest.mjs tests/Unit/ServerParityWorkerListingContractTest.mjs tests/Unit/ServerParityWorkerReplacementContractTest.mjs tests/Unit/ServerParityPatchDeploymentContractTest.mjs tests/Unit/ServerParityWaitingHistoryContractTest.mjs tests/Unit/ServerParityTaskRetryContractTest.mjs tests/Unit/ServerParityPublishedPatchContractTest.mjs tests/Unit/ServerParityPublishedPatchTransportTest.mjs tests/Unit/ServerParityRecordFileTest.mjs tests/Unit/ServerParityFailureLocationTest.mjs tests/Unit/ServerParityFailureExcerptTest.mjs tests/Unit/ServerParityLateWorkflowCompletionContractTest.mjs tests/Unit/ServerParityStorageTestContractTest.mjs tests/Unit/ServerParitySchedulePoliciesContractTest.mjs tests/Unit/ServerParitySignalsBeforeWorkerContractTest.mjs tests/Unit/ServerParityScheduleCatalogContractTest.mjs tests/Unit/ServerParityOperatorMetricsContractTest.mjs tests/Unit/ServerParityActivityLeaseRenewalContractTest.mjs tests/Unit/ServerParityConcurrentWaitsContractTest.mjs tests/Unit/ServerParityContinueTransfersContractTest.mjs tests/Unit/ServerParityLargeCommandBatchesContractTest.mjs tests/Unit/ServerParityManyUpdatesContractTest.mjs tests/Unit/ServerParityStartSearchAttributesContractTest.mjs tests/Unit/ServerParityQueryArgumentsContractTest.mjs tests/Unit/ServerParityQueryContractArgumentsContractTest.mjs tests/Unit/ServerParityParentCloseCancellationContractTest.mjs tests/Unit/ServerParityRecordLocalActivityContractTest.mjs tests/Unit/ServerParityActivityCancellationAckContractTest.mjs tests/Unit/ServerParityServiceCatalogContractTest.mjs tests/Unit/ServerParityServiceEndpointsContractTest.mjs tests/Unit/ServerParityServiceCallsContractTest.mjs tests/Unit/ServerParityWorkflowImportContractTest.mjs tests/Unit/ServerParityWaterlineImportContractTest.mjs tests/Unit/ServerParitySystemMetricsContractTest.mjs tests/Unit/ServerParityOperatorDashboardTypesContractTest.mjs tests/Unit/ServerParityPrometheusMetricsContractTest.mjs tests/Unit/ServerParityPayloadCleanupContractTest.mjs tests/Unit/ServerParityRepairPassContractTest.mjs tests/Unit/ServerParityClusterInfoContractTest.mjs tests/Unit/ServerParityNamespaceDeleteContractTest.mjs tests/Unit/ServerParityMessageStreamsContractTest.mjs tests/Unit/ServerParityCancellationScopesContractTest.mjs tests/Unit/ServerParityWorkflowPollRequestContractTest.mjs tests/Unit/ServerParityUpdateValidationContractTest.mjs tests/Unit/ServerParityFixtureSelectionTest.mjs tests/Unit/ServerParityUpdateValidationRetryTest.mjs tests/Unit/ServerParityKilledAuthorPollTest.mjs tests/Unit/ServerParityImagePullTest.mjs tests/Unit/ServerParityAdapterCacheTest.mjs tests/Unit/ServerParityWorkerSessionsContractTest.mjs tests/Unit/ServerParityWorkerSessionActivitiesContractTest.mjs tests/Unit/ServerParityNexusOperationsContractTest.mjs tests/Unit/ServerParityBridgeAdaptersContractTest.mjs tests/Unit/ServerParityWorkflowStreamWritesContractTest.mjs > parity-evidence/comparator-tests.txt + "$image" --test tests/Unit/ServerParityRunnerTest.mjs tests/Unit/ServerParityRunnerBlockedExcerptTest.mjs tests/Unit/ServerParityProbeNotesTest.mjs tests/Unit/ServerParitySdkDirectionLanesTest.mjs tests/Unit/ServerParityCodecDeadlineCellsTest.mjs tests/Unit/ServerParityCooperativeContractTest.mjs tests/Unit/ServerParityChildCancellationContractTest.mjs tests/Unit/ServerParityScheduleContractTest.mjs tests/Unit/ServerParityVisibilityContractTest.mjs tests/Unit/ServerParityAdmissionContractTest.mjs tests/Unit/ServerParityRoleTokenContractTest.mjs tests/Unit/ServerParityWorkerDeregistrationContractTest.mjs tests/Unit/ServerParitySearchAttributeContractTest.mjs tests/Unit/ServerParityWorkerListingContractTest.mjs tests/Unit/ServerParityWorkerReplacementContractTest.mjs tests/Unit/ServerParityPatchDeploymentContractTest.mjs tests/Unit/ServerParityWaitingHistoryContractTest.mjs tests/Unit/ServerParityTaskRetryContractTest.mjs tests/Unit/ServerParityPublishedPatchContractTest.mjs tests/Unit/ServerParityPublishedPatchTransportTest.mjs tests/Unit/ServerParityRecordFileTest.mjs tests/Unit/ServerParityFailureLocationTest.mjs tests/Unit/ServerParityFailureExcerptTest.mjs tests/Unit/ServerParityLateWorkflowCompletionContractTest.mjs tests/Unit/ServerParityStorageTestContractTest.mjs tests/Unit/ServerParitySchedulePoliciesContractTest.mjs tests/Unit/ServerParitySignalsBeforeWorkerContractTest.mjs tests/Unit/ServerParityScheduleCatalogContractTest.mjs tests/Unit/ServerParityOperatorMetricsContractTest.mjs tests/Unit/ServerParityActivityLeaseRenewalContractTest.mjs tests/Unit/ServerParityConcurrentWaitsContractTest.mjs tests/Unit/ServerParityContinueTransfersContractTest.mjs tests/Unit/ServerParityWorkerErrorBodiesContractTest.mjs tests/Unit/ServerParityLargeCommandBatchesContractTest.mjs tests/Unit/ServerParityManyUpdatesContractTest.mjs tests/Unit/ServerParityStartSearchAttributesContractTest.mjs tests/Unit/ServerParityQueryArgumentsContractTest.mjs tests/Unit/ServerParityQueryContractArgumentsContractTest.mjs tests/Unit/ServerParityParentCloseCancellationContractTest.mjs tests/Unit/ServerParityRecordLocalActivityContractTest.mjs tests/Unit/ServerParityActivityCancellationAckContractTest.mjs tests/Unit/ServerParityServiceCatalogContractTest.mjs tests/Unit/ServerParityServiceEndpointsContractTest.mjs tests/Unit/ServerParityServiceCallsContractTest.mjs tests/Unit/ServerParityWorkflowImportContractTest.mjs tests/Unit/ServerParityWaterlineImportContractTest.mjs tests/Unit/ServerParitySystemMetricsContractTest.mjs tests/Unit/ServerParityOperatorDashboardTypesContractTest.mjs tests/Unit/ServerParityPrometheusMetricsContractTest.mjs tests/Unit/ServerParityPayloadCleanupContractTest.mjs tests/Unit/ServerParityRepairPassContractTest.mjs tests/Unit/ServerParityClusterInfoContractTest.mjs tests/Unit/ServerParityNamespaceDeleteContractTest.mjs tests/Unit/ServerParityMessageStreamsContractTest.mjs tests/Unit/ServerParityCancellationScopesContractTest.mjs tests/Unit/ServerParityWorkflowPollRequestContractTest.mjs tests/Unit/ServerParityUpdateValidationContractTest.mjs tests/Unit/ServerParityFixtureSelectionTest.mjs tests/Unit/ServerParityUpdateValidationRetryTest.mjs tests/Unit/ServerParityKilledAuthorPollTest.mjs tests/Unit/ServerParityImagePullTest.mjs tests/Unit/ServerParityAdapterCacheTest.mjs tests/Unit/ServerParityWorkerSessionsContractTest.mjs tests/Unit/ServerParityWorkerSessionActivitiesContractTest.mjs tests/Unit/ServerParityNexusOperationsContractTest.mjs tests/Unit/ServerParityBridgeAdaptersContractTest.mjs tests/Unit/ServerParityWorkflowStreamWritesContractTest.mjs > parity-evidence/comparator-tests.txt docker run --rm --user=1000:1000 --cpus=1 --memory=256m --memory-swap=256m \ --network=none --entrypoint node -v "$PWD:/source:ro" -w /source \ "$image" --test tests/Unit/ServerParityCodecDeadlineLoadTest.mjs > parity-evidence/codec-load-tests.txt diff --git a/docs/rust-server-port-checkpoints/2026-10-29-worker-error-bodies.md b/docs/rust-server-port-checkpoints/2026-10-29-worker-error-bodies.md new file mode 100644 index 000000000..b93232156 --- /dev/null +++ b/docs/rust-server-port-checkpoints/2026-10-29-worker-error-bodies.md @@ -0,0 +1,48 @@ +# October 29, worker-plane error bodies + +- **Before.** Sending the same malformed worker requests to published + 2.11.5 and native showed three differences: + - a workflow or activity poll that failed its request rules was answered + with native's own reasons (`422 invalid_request`, `invalid_poll_timeout` + and the like), not Laravel's `validation_failed`; + - failing or heartbeating an activity task the namespace does not hold was + answered with a bare `{message, reason}` outside the worker envelope; + - completing a workflow task the namespace does not hold with a + `complete_workflow` command without a result was answered + `422 unsupported_payload_codec`, where PHP answers `404 task_not_found`. + + Validation failures on other worker routes, and unknown routes and wrong + methods in the default namespace, already matched. +- **Contract** (published 2.11.5; `bootstrap/app.php` exception rendering + with `WorkerProtocol::json`, `WorkerController::pollWorkflowTasks`, + `ActivityTaskController::poll`, `guardAttemptOwnership`): + - a worker-plane validation failure answers `422 validation_failed` with + Laravel's messages in `errors` and `validation_errors`, plus + `protocol_version` and `server_capabilities`; + - the workflow poll's rules are `worker_id` and `task_queue` (required + strings), `build_id`, `poll_request_id` (at most 255), + `timeout_seconds` (0 to 60), `history_page_size` (1 to 1000), + `accept_history_encoding` (at most 64) and `task_kinds` (1 or 2 of + `workflow` and `update_validation`, distinct). The activity poll takes + the first five; + - completing, failing or heartbeating an activity task the namespace does + not hold answers `404` with `task_id`, `activity_attempt_id`, + `error: "Activity task not found."` and `reason: task_not_found`, in the + envelope; + - completing a workflow task the namespace does not hold answers `404` + with `task_id`, `workflow_task_attempt` and "Workflow task not found." + The ownership guard runs before the command normalizer reads payloads. +- **Fixture.** `worker-error-bodies`, raw worker HTTP on protocol 1.20 in a + fresh namespace: ten requests. The probe records each status and body, + with `server_capabilities` reduced to whether it is present. +- **Native.** `http.rs` validates both polls with the published rules before + reading them (native's task-kinds reader still checks distinct values), + and answers an unknown activity task on the fail and heartbeat routes as + the complete route already did. In a named namespace, a request PHP has + no route for used to be refused as an unqualified named-namespace + operation (`422 development_namespace_operation_unqualified`); it now gets + PHP's 404 or 405, as in the default namespace. Only operations PHP serves + are still refused that way. A workflow task completion or checkpoint now + looks the task up before checking or fetching any command payload. +- **Tests.** `worker_error_bodies.rs` fails on the previous source and + passes now. diff --git a/docs/rust-server-port.md b/docs/rust-server-port.md index fc0a49070..c1bf223ee 100644 --- a/docs/rust-server-port.md +++ b/docs/rust-server-port.md @@ -666,11 +666,6 @@ contract fixture before its correction. - **A repeated identical activity result.** Published PHP answers a repeat of the result that finished an activity task with `409 stale_attempt`. Native returns its earlier idempotent receipt (`200`, `already_completed`). -- **Worker-plane error bodies.** Published PHP adds `protocol_version` to - every worker-plane response, including validation failures, and answers an - unknown activity task with `task_id`, `activity_attempt_id`, `error` and - `reason`. Native's worker-plane errors carry only Laravel's validation - fields or `message` and `reason`. - **Authentication beyond role tokens and runtime credentials.** Native refuses principal tokens (`DW_PRINCIPAL_TOKENS`), custom providers and the signature driver at startup. It still requires `DW_AUTH_TOKEN` with the diff --git a/rust/src/runtime/execution.rs b/rust/src/runtime/execution.rs index f6c73df8a..20963934d 100644 --- a/rust/src/runtime/execution.rs +++ b/rust/src/runtime/execution.rs @@ -1546,6 +1546,16 @@ impl Runtime { protocol: Option<&str>, checkpoint: Option<&super::scope_checkpoint::Checkpoint>, ) -> Result { + // A task the namespace does not hold is answered before any payload + // is read or fetched, as PHP's guards run before its normalizer. + let held = match &*self.storage { + Storage::Sqlite(store) => store.task_held(task_id).await?, + Storage::Postgres(store) => store.task_held(task_id).await?, + Storage::MySql(store) => store.task_held(task_id).await?, + }; + if !held { + return Err(refuse(StatusCode::NOT_FOUND, "task_not_found")); + } self.check_command_payloads(&body).await?; let external = self.prepare_command_payloads(&body).await?; let body = super::completion_rules::canonical_payloads(body)?; diff --git a/rust/src/runtime/http.rs b/rust/src/runtime/http.rs index 6ab2f927d..df0a6075a 100644 --- a/rust/src/runtime/http.rs +++ b/rust/src/runtime/http.rs @@ -1,3 +1,4 @@ +use super::laravel_rules::Rule; use axum::{ Extension, Json, Router, extract::{DefaultBodyLimit, Path, Query, Request, State}, @@ -655,6 +656,13 @@ async fn guard(State(runtime): State, mut request: Request, next: Next) return admission_error(request, worker, StatusCode::NOT_FOUND, body).await; } if namespace != "default" && !named_namespace_operation(request.method().as_str(), &path) { + // A request PHP has no route for gets PHP's 404 or 405 in any + // namespace; only an operation PHP serves is refused here. + if !matches!(super::route_fallback::answer(request.method().as_str(), &path), + super::route_fallback::Answer::NotImplemented) + { + return route_fallback(request.method().clone(), request.uri().clone()).await; + } return admission_error(request,worker,StatusCode::UNPROCESSABLE_ENTITY, json!({"reason":"development_namespace_operation_unqualified","namespace":namespace, "message":"This operation is not implemented for named namespaces in the unpublished execution slice."})).await; @@ -2088,13 +2096,44 @@ async fn deregister_fenced( } } } +/// `WorkerController::pollWorkflowTasks`' request rules. Native checks +/// `task_kinds.*` values and their distinctness when it reads them. +const WORKFLOW_POLL_RULES: &super::laravel_rules::Rules = &[ + ("worker_id", &[Rule::Required, Rule::String]), + ("task_queue", &[Rule::Required, Rule::String]), + ("build_id", &[Rule::Nullable, Rule::String]), + ("poll_request_id", &[Rule::Nullable, Rule::String, Rule::Max(255)]), + ("timeout_seconds", &[Rule::Nullable, Rule::Integer, Rule::Min(0), Rule::Max(60)]), + ("history_page_size", &[Rule::Nullable, Rule::Integer, Rule::Min(1), Rule::Max(1000)]), + ("accept_history_encoding", &[Rule::Nullable, Rule::String, Rule::Max(64)]), + ("task_kinds", &[Rule::Nullable, Rule::Array, Rule::Min(1), Rule::Max(2)]), + ("task_kinds.*", &[Rule::String, Rule::In(&["workflow", "update_validation"])]), +]; + +/// `ActivityTaskController::poll`'s request rules. +const ACTIVITY_POLL_RULES: &super::laravel_rules::Rules = &[ + ("worker_id", &[Rule::Required, Rule::String]), + ("task_queue", &[Rule::Required, Rule::String]), + ("build_id", &[Rule::Nullable, Rule::String]), + ("poll_request_id", &[Rule::Nullable, Rule::String, Rule::Max(255)]), + ("timeout_seconds", &[Rule::Nullable, Rule::Integer, Rule::Min(0), Rule::Max(60)]), +]; + +/// A poll's request rules, answered as PHP's worker plane answers a +/// validation failure: in the worker envelope. +fn poll_rules(body: &Value, rules: &super::laravel_rules::Rules) -> Result<()> { + super::laravel_rules::validate(&super::laravel_rules::normalize(body), rules).map_err(worker_refusal) +} + async fn poll_workflow( Extension(runtime): Extension, headers: HeaderMap, Json(body): Json, ) -> Result> { + let protocol = request_protocol(&headers)?; + poll_rules(&body, WORKFLOW_POLL_RULES)?; runtime - .poll_workflow_tasks(body, request_protocol(&headers)?) + .poll_workflow_tasks(body, protocol) .await .map(Json) .map_err(worker_refusal) @@ -2139,9 +2178,11 @@ async fn poll_activity( headers: HeaderMap, Json(body): Json, ) -> Result> { + let protocol = request_protocol(&headers)?; + poll_rules(&body, ACTIVITY_POLL_RULES)?; let wait = runtime.worker_poll_wait(); runtime - .poll_with_pages(body, "activity", request_protocol(&headers)?, Default::default(), wait) + .poll_with_pages(body, "activity", protocol, Default::default(), wait) .await .map(Json) } @@ -2380,12 +2421,26 @@ async fn heartbeat_activity( headers: HeaderMap, Json(body): Json, ) -> Result> { + let attempt = body["activity_attempt_id"].clone(); runtime .heartbeat_activity(&task_id, body, request_protocol(&headers)?) .await + .map_err(|error| activity_not_found(error, &task_id, attempt)) .map(Json) } +/// `guardAttemptOwnership`'s answer for a task the namespace does not hold, +/// in the worker envelope, as the fail and heartbeat routes give it. +fn activity_not_found(error: super::RuntimeError, task_id: &str, attempt: Value) -> super::RuntimeError { + match error { + super::RuntimeError::Refused { status, reason: "task_not_found" } if status == StatusCode::NOT_FOUND => { + super::RuntimeError::Protocol { status, response: worker_response(json!({"task_id":task_id, + "activity_attempt_id":attempt,"error":"Activity task not found.","reason":"task_not_found"})) } + } + other => other, + } +} + async fn renew_activity_lease( Extension(runtime): Extension, Path(task_id): Path, @@ -2470,7 +2525,12 @@ async fn fail_activity( Path(task_id): Path, Json(body): Json, ) -> Result> { - runtime.fail_activity(&task_id, body).await.map(|body| Json(worker_response(body))) + let attempt = body["activity_attempt_id"].clone(); + runtime + .fail_activity(&task_id, body) + .await + .map_err(|error| activity_not_found(error, &task_id, attempt)) + .map(|body| Json(worker_response(body))) } /// The query's command context, as PHP's controller builds it. fn query_context(request: &CommandRequest, body: &Value, workflow_id: &str, name: &str) -> Value { diff --git a/rust/src/runtime/store.rs b/rust/src/runtime/store.rs index eeeebf00c..29cf510a5 100644 --- a/rust/src/runtime/store.rs +++ b/rust/src/runtime/store.rs @@ -3939,6 +3939,17 @@ where self.wake.notify_waiters(); Ok(()) } + /// Whether the namespace holds the task: PHP's ownership guard looks it + /// up before the command normalizer reads any payload. + pub(super) async fn task_held(&self, task_id: &str) -> Result { + Ok(Self::scalar("SELECT COUNT(*) FROM workflow_tasks WHERE id=$1 AND namespace=$2") + .bind(task_id) + .bind(self.namespace.as_ref()) + .fetch_one(&self.pool) + .await? + > 0) + } + pub(super) async fn task_row( &self, tx: &mut Transaction<'_, DB>, diff --git a/rust/tests/execution.rs b/rust/tests/execution.rs index 482ece65b..5f15f7c8b 100644 --- a/rust/tests/execution.rs +++ b/rust/tests/execution.rs @@ -350,6 +350,8 @@ mod close_inventory; #[path = "execution/start_search_attributes.rs"] mod start_search_attributes; +#[path = "execution/worker_error_bodies.rs"] +mod worker_error_bodies; enum TestDatabase { Sqlite(tempfile::TempDir), diff --git a/rust/tests/execution/worker_error_bodies.rs b/rust/tests/execution/worker_error_bodies.rs new file mode 100644 index 000000000..23cbf1e7f --- /dev/null +++ b/rust/tests/execution/worker_error_bodies.rs @@ -0,0 +1,99 @@ +use super::*; + +// Published Server 2.11.5 answers worker-plane validation failures and +// unknown activity tasks in the worker envelope. The worker-error-bodies +// fixture records the same requests. + +fn enveloped(body: &Value) -> bool { + body["protocol_version"].is_string() && body["server_capabilities"].is_object() +} + +#[tokio::test] +async fn poll_validation_failures_carry_laravels_messages_in_the_envelope() { + let database = TestDatabase::new().await; + let runtime = database.open().await.unwrap(); + let app = router(runtime.clone()); + let (status, body) = request_protocol(&app, "POST", "/api/worker/workflow-tasks/poll", json!({}), "1.20").await; + assert_eq!((status, body["reason"].clone()), (StatusCode::UNPROCESSABLE_ENTITY, json!("validation_failed")), "{body}"); + assert_eq!(body["errors"], json!({"worker_id":["The worker id field is required."],"task_queue":["The task queue field is required."]})); + assert_eq!(body["validation_errors"], body["errors"]); + assert_eq!(body["message"], "The worker id field is required. (and 1 more error)"); + assert!(enveloped(&body), "{body}"); + let (status, body) = request_protocol(&app, "POST", "/api/worker/workflow-tasks/poll", json!({"worker_id":"w","task_queue":"q", + "timeout_seconds":61,"history_page_size":0,"poll_request_id":"r".repeat(256),"task_kinds":[]}), "1.20").await; + assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY, "{body}"); + assert_eq!(body["message"], "The poll request id field must not be greater than 255 characters. (and 3 more errors)"); + assert_eq!(body["errors"]["task_kinds"], json!(["The task kinds field must have at least 1 items."])); + let (status, body) = request_protocol(&app, "POST", "/api/worker/activity-tasks/poll", + json!({"worker_id":"w","task_queue":"q","timeout_seconds":-1}), "1.20").await; + assert_eq!((status, body["errors"].clone()), (StatusCode::UNPROCESSABLE_ENTITY, + json!({"timeout_seconds":["The timeout seconds field must be at least 0."]})), "{body}"); + assert!(enveloped(&body), "{body}"); + runtime.close().await; + database.remove().await; +} + +#[tokio::test] +async fn an_unknown_activity_task_is_answered_with_its_identity_on_every_route() { + let database = TestDatabase::new().await; + let runtime = database.open().await.unwrap(); + let app = router(runtime.clone()); + let attempt = json!({"activity_attempt_id":"missing-attempt","lease_owner":"missing-worker"}); + for (route, extra) in [ + ("complete", json!({"result":envelope(Payload::Null)})), + ("fail", json!({"failure":{"message":"missing"}})), + ("heartbeat", json!({})), + ] { + let mut body = attempt.clone(); + body.as_object_mut().unwrap().extend(extra.as_object().unwrap().clone()); + let (status, answer) = request_protocol(&app, "POST", &format!("/api/worker/activity-tasks/missing-task/{route}"), body, "1.20").await; + assert_eq!(status, StatusCode::NOT_FOUND, "{route}: {answer}"); + assert_eq!((answer["task_id"].clone(), answer["activity_attempt_id"].clone(), answer["error"].clone(), answer["reason"].clone()), + (json!("missing-task"), json!("missing-attempt"), json!("Activity task not found."), json!("task_not_found")), "{route}: {answer}"); + assert!(enveloped(&answer), "{route}: {answer}"); + } + runtime.close().await; + database.remove().await; +} + +// PHP's 404 and 405 do not depend on the namespace: an unknown worker route +// in a named namespace is not refused as an unqualified named operation. +#[tokio::test] +async fn unknown_worker_routes_answer_404_and_405_in_a_named_namespace() { + let database = TestDatabase::new().await; + let runtime = database.open().await.unwrap(); + let app = router(runtime.clone()); + let (status, body) = request(&app, "POST", "/api/namespaces", json!({"name":"errors-elsewhere","description":"named"})).await; + assert!(status.is_success(), "{body}"); + for (method, path, expected) in [ + ("GET", "/api/worker/nothing-here", StatusCode::NOT_FOUND), + ("GET", "/api/worker/workflow-tasks/poll", StatusCode::METHOD_NOT_ALLOWED), + ] { + let response = app.clone().oneshot(Request::builder().method(method).uri(path) + .header("authorization", "Bearer test-token").header("x-namespace", "errors-elsewhere") + .header("x-durable-workflow-protocol-version", "1.20").body(Body::empty()).unwrap()).await.unwrap(); + assert_eq!(response.status(), expected, "{method} {path}"); + let body: Value = serde_json::from_slice(&axum::body::to_bytes(response.into_body(), usize::MAX).await.unwrap()).unwrap(); + assert!(enveloped(&body), "{body}"); + } + runtime.close().await; + database.remove().await; +} + +// PHP's ownership guard finds an unknown workflow task before its command +// normalizer reads a payload: a result-less complete_workflow is not +// refused first. +#[tokio::test] +async fn an_unknown_workflow_task_is_answered_before_its_payloads_are_read() { + let database = TestDatabase::new().await; + let runtime = database.open().await.unwrap(); + let app = router(runtime.clone()); + let (status, body) = request_protocol(&app, "POST", "/api/worker/workflow-tasks/missing-task/complete", + json!({"lease_owner":"missing-worker","workflow_task_attempt":1,"commands":[{"type":"complete_workflow"}]}), "1.20").await; + assert_eq!(status, StatusCode::NOT_FOUND, "{body}"); + assert_eq!((body["task_id"].clone(), body["workflow_task_attempt"].clone(), body["error"].clone(), body["reason"].clone()), + (json!("missing-task"), json!(1), json!("Workflow task not found."), json!("task_not_found")), "{body}"); + assert!(enveloped(&body), "{body}"); + runtime.close().await; + database.remove().await; +} diff --git a/scripts/conformance/server-parity/contract.mjs b/scripts/conformance/server-parity/contract.mjs index bd1985d57..ac81f43c7 100644 --- a/scripts/conformance/server-parity/contract.mjs +++ b/scripts/conformance/server-parity/contract.mjs @@ -17,6 +17,7 @@ import {checkConcurrentWaits} from './concurrent-waits-contract.mjs'; import {checkContinueTransfers} from './continue-transfers-contract.mjs'; import {checkLargeCommandBatches} from './large-command-batches-contract.mjs'; import {checkManyUpdates} from './many-updates-contract.mjs'; +import {checkWorkerErrorBodies} from './worker-error-bodies-contract.mjs'; import {checkStartSearchAttributes} from './start-search-attributes-contract.mjs'; import {checkQueryArgumentRouting} from './query-arguments-contract.mjs'; import {checkQueryContractArguments} from './query-contract-arguments-contract.mjs'; @@ -179,6 +180,7 @@ export function checkObservation(fixture, observation, workflowId, artifacts) { const continueTransfers = fixture.continue_transfers ? checkContinueTransfers(fixture, observation) : null; const largeCommandBatches = fixture.large_command_batches ? checkLargeCommandBatches(fixture, observation) : null; const manyUpdates = fixture.many_updates ? checkManyUpdates(fixture, observation) : null; + const workerErrorBodies = fixture.worker_error_bodies ? checkWorkerErrorBodies(fixture, observation) : null; const startSearchAttributes = fixture.start_search_attributes ? checkStartSearchAttributes(fixture, observation) : null; const queryArguments = fixture.query_argument_routing ? checkQueryArgumentRouting(fixture, observation) : null; const queryContractArguments = fixture.query_contract_arguments ? checkQueryContractArguments(fixture, observation) : null; @@ -759,6 +761,7 @@ export function checkObservation(fixture, observation, workflowId, artifacts) { ...(continueTransfers ? {continue_transfers: continueTransfers} : {}), ...(largeCommandBatches ? {large_command_batches: largeCommandBatches} : {}), ...(manyUpdates ? {many_updates: manyUpdates} : {}), + ...(workerErrorBodies ? {worker_error_bodies: workerErrorBodies} : {}), ...(startSearchAttributes ? {start_search_attributes: startSearchAttributes} : {}), ...(queryArguments ? {query_argument_routing: queryArguments} : {}), ...(queryContractArguments ? {query_contract_arguments: queryContractArguments} : {}), diff --git a/scripts/conformance/server-parity/probe.php b/scripts/conformance/server-parity/probe.php index 2659a4b5c..0790284be 100644 --- a/scripts/conformance/server-parity/probe.php +++ b/scripts/conformance/server-parity/probe.php @@ -435,6 +435,7 @@ function httpObservation(array $fixture, string $workflowId, string $namespace, $continueTransfers = isset($fixture['continue_transfers']) ? finishHttpContinueTransfers($client, $fixture, $workflowId, $url) : null; $largeCommandBatches = isset($fixture['large_command_batches']) ? finishHttpLargeCommandBatches($client, $fixture, $workflowId, $url) : null; $manyUpdates = isset($fixture['many_updates']) ? finishHttpManyUpdates($client, $fixture, $workflowId, $url) : null; + $workerErrorBodies = isset($fixture['worker_error_bodies']) ? finishHttpWorkerErrorBodies($client, $fixture, $workflowId, $url) : null; $startSearchAttributes = isset($fixture['start_search_attributes']) ? finishHttpStartSearchAttributes($client, $fixture, $workflowId, $url) : null; $queryArgumentRouting = isset($fixture['query_argument_routing']) ? finishHttpQueryArgumentRouting($client, $fixture, $workflowId, $url) : null; $queryContractArguments = isset($fixture['query_contract_arguments']) ? finishHttpQueryContractArguments($client, $fixture, $workflowId, $url) : null; @@ -568,6 +569,7 @@ function httpObservation(array $fixture, string $workflowId, string $namespace, 'continue_transfers' => $continueTransfers, 'large_command_batches' => $largeCommandBatches, 'many_updates' => $manyUpdates, + 'worker_error_bodies' => $workerErrorBodies, 'start_search_attributes' => $startSearchAttributes, 'query_argument_routing' => $queryArgumentRouting, 'query_contract_arguments' => $queryContractArguments, @@ -904,6 +906,7 @@ function embeddedObservation(array $fixture, string $workflowId, string $namespa require __DIR__.'/continue-transfers-probe.php'; require __DIR__.'/large-command-batches-probe.php'; require __DIR__.'/many-updates-probe.php'; + require __DIR__.'/worker-error-bodies-probe.php'; require __DIR__.'/start-search-attributes-probe.php'; require __DIR__.'/query-arguments-probe.php'; require __DIR__.'/query-contract-arguments-probe.php'; @@ -1102,6 +1105,9 @@ function embeddedObservation(array $fixture, string $workflowId, string $namespa if ($mode === 'embedded' && isset($fixture['many_updates'])) { $observation['many_updates'] = ['applicable' => false, 'reason' => 'embedded_has_no_worker_protocol']; } + if ($mode === 'embedded' && isset($fixture['worker_error_bodies'])) { + $observation['worker_error_bodies'] = ['applicable' => false, 'reason' => 'embedded_has_no_worker_protocol']; + } if ($mode === 'embedded' && isset($fixture['start_search_attributes'])) { $observation['start_search_attributes'] = ['applicable' => false, 'reason' => 'embedded_has_no_http_search_attribute_registry']; } diff --git a/scripts/conformance/server-parity/worker-error-bodies-contract.mjs b/scripts/conformance/server-parity/worker-error-bodies-contract.mjs new file mode 100644 index 000000000..8d7b35b7c --- /dev/null +++ b/scripts/conformance/server-parity/worker-error-bodies-contract.mjs @@ -0,0 +1,24 @@ +import assert from 'node:assert/strict'; + +// Reviewed worker-plane error bodies (worker-error-bodies-probe.php), +// recorded against published Server 2.11.5: poll validation failures, +// unknown activity tasks and unknown routes or methods, each in the worker +// envelope. The fixture's `expected` holds the whole observation. +export function checkWorkerErrorBodies(fixture, observation) { + const definition = fixture.worker_error_bodies; + const state = observation.worker_error_bodies; + const projection = {scope: 'worker_error_bodies', requests: Object.keys(definition.requests).length}; + assert.ok(state && typeof state === 'object', 'actual worker error bodies observation'); + if (observation.mode === 'embedded') { + assert.deepStrictEqual(state, {applicable: false, reason: 'embedded_has_no_worker_protocol'}, + 'embedded worker error bodies explicitly inapplicable'); + return projection; + } + assert.equal(state.applicable, true, 'HTTP targets record worker error bodies'); + for (const [request, expected] of Object.entries(definition.expected)) { + assert.deepStrictEqual(state[request], expected, `worker error bodies ${request}`); + } + assert.deepStrictEqual(Object.keys(state).sort(), ['applicable', ...Object.keys(definition.expected)].sort(), + 'worker error bodies requests'); + return projection; +} diff --git a/scripts/conformance/server-parity/worker-error-bodies-probe.php b/scripts/conformance/server-parity/worker-error-bodies-probe.php new file mode 100644 index 000000000..152f23d26 --- /dev/null +++ b/scripts/conformance/server-parity/worker-error-bodies-probe.php @@ -0,0 +1,38 @@ +createNamespace($namespace, 'worker error bodies λ', 30); + $token = getenv('DW_PARITY_TOKEN') ?: null; + $http = new WorkerErrorBodiesHttp(['base_uri' => rtrim($url, '/').'/api/', 'http_errors' => false, + 'connect_timeout' => 1, 'timeout' => 30]); + $headers = array_filter([ + 'Authorization' => $token === null ? null : 'Bearer '.$token, + 'Accept' => 'application/json', 'X-Namespace' => $namespace, + 'X-Durable-Workflow-Protocol-Version' => '1.20', + ]); + $state = ['applicable' => true]; + foreach ($definition['requests'] as $case => [$method, $path, $body]) { + $response = $http->request($method, $path, ['headers' => $headers, + ...($body === null ? [] : ['json' => $body === [] ? (object) [] : $body])]); + $decoded = json_decode((string) $response->getBody(), true, flags: JSON_THROW_ON_ERROR); + $hasCapabilities = is_array($decoded['server_capabilities'] ?? null); + unset($decoded['server_capabilities']); + ksort($decoded); + $state[$case] = ['status' => $response->getStatusCode(), 'server_capabilities' => $hasCapabilities, 'body' => $decoded]; + } + + return $state; +} diff --git a/tests/Fixtures/ServerParity/worker-error-bodies.json b/tests/Fixtures/ServerParity/worker-error-bodies.json new file mode 100644 index 000000000..650bcd271 --- /dev/null +++ b/tests/Fixtures/ServerParity/worker-error-bodies.json @@ -0,0 +1,294 @@ +{ + "schema": "durable-workflow.server-parity-fixture/v1", + "id": "worker-error-bodies", + "review_state": "reviewed", + "contract": "Worker-plane error bodies (published Server 2.11.5 bootstrap/app.php exception rendering with WorkerProtocol::json, WorkerController::pollWorkflowTasks and ActivityTaskController poll, fail and heartbeat), over raw worker HTTP on protocol 1.20 in a fresh namespace. A workflow or activity poll that fails its request rules answers 422 validation_failed with Laravel's messages in errors and validation_errors, in the worker envelope (protocol_version and server_capabilities). Completing, failing or heartbeating an activity task the namespace does not hold answers 404 task_not_found with the task id, attempt id and 'Activity task not found.' in the envelope. Completing a workflow task the namespace does not hold answers 404 task_not_found with 'Workflow task not found.' before any command payload is read. An unknown worker route answers 404 not_found and a wrong method 405 method_not_allowed, both in the envelope. Embedded applications have no worker protocol.", + "workflow_type": "parity.v1.echo", + "input": { + "message": "continue transfers λ", + "count": 9007199254740993, + "optional": null + }, + "typed_value": { + "type": "map", + "value": { + "message": { + "type": "string", + "value": "continue transfers λ" + }, + "count": { + "type": "int64", + "value": "9007199254740993" + }, + "optional": { + "type": "null", + "value": null + } + } + }, + "activity": false, + "expected_events": [ + "StartAccepted", + "WorkflowStarted", + "WorkflowCompleted" + ], + "worker_error_bodies": { + "requests": { + "workflow_poll_empty": [ + "POST", + "worker/workflow-tasks/poll", + {} + ], + "workflow_poll_out_of_range": [ + "POST", + "worker/workflow-tasks/poll", + { + "worker_id": "w", + "task_queue": "q", + "timeout_seconds": 61, + "history_page_size": 0, + "poll_request_id": "rrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrr", + "task_kinds": [] + } + ], + "activity_poll_empty": [ + "POST", + "worker/activity-tasks/poll", + {} + ], + "activity_poll_out_of_range": [ + "POST", + "worker/activity-tasks/poll", + { + "worker_id": "w", + "task_queue": "q", + "timeout_seconds": -1 + } + ], + "activity_complete_unknown": [ + "POST", + "worker/activity-tasks/missing-task/complete", + { + "activity_attempt_id": "missing-attempt", + "lease_owner": "missing-worker", + "result": { + "codec": "avro", + "blob": "wwHioz3/VYAiNwwA" + } + } + ], + "activity_fail_unknown": [ + "POST", + "worker/activity-tasks/missing-task/fail", + { + "activity_attempt_id": "missing-attempt", + "lease_owner": "missing-worker", + "failure": { + "message": "missing" + } + } + ], + "activity_heartbeat_unknown": [ + "POST", + "worker/activity-tasks/missing-task/heartbeat", + { + "activity_attempt_id": "missing-attempt", + "lease_owner": "missing-worker" + } + ], + "route_not_found": [ + "GET", + "worker/nothing-here", + null + ], + "method_not_allowed": [ + "GET", + "worker/workflow-tasks/poll", + null + ], + "workflow_complete_unknown": [ + "POST", + "worker/workflow-tasks/missing-task/complete", + { + "lease_owner": "missing-worker", + "workflow_task_attempt": 1, + "commands": [ + { + "type": "complete_workflow" + } + ] + } + ] + }, + "expected": { + "workflow_poll_empty": { + "status": 422, + "server_capabilities": true, + "body": { + "errors": { + "worker_id": [ + "The worker id field is required." + ], + "task_queue": [ + "The task queue field is required." + ] + }, + "message": "The worker id field is required. (and 1 more error)", + "protocol_version": "1.20", + "reason": "validation_failed", + "validation_errors": { + "worker_id": [ + "The worker id field is required." + ], + "task_queue": [ + "The task queue field is required." + ] + } + } + }, + "workflow_poll_out_of_range": { + "status": 422, + "server_capabilities": true, + "body": { + "errors": { + "poll_request_id": [ + "The poll request id field must not be greater than 255 characters." + ], + "timeout_seconds": [ + "The timeout seconds field must not be greater than 60." + ], + "history_page_size": [ + "The history page size field must be at least 1." + ], + "task_kinds": [ + "The task kinds field must have at least 1 items." + ] + }, + "message": "The poll request id field must not be greater than 255 characters. (and 3 more errors)", + "protocol_version": "1.20", + "reason": "validation_failed", + "validation_errors": { + "poll_request_id": [ + "The poll request id field must not be greater than 255 characters." + ], + "timeout_seconds": [ + "The timeout seconds field must not be greater than 60." + ], + "history_page_size": [ + "The history page size field must be at least 1." + ], + "task_kinds": [ + "The task kinds field must have at least 1 items." + ] + } + } + }, + "activity_poll_empty": { + "status": 422, + "server_capabilities": true, + "body": { + "errors": { + "worker_id": [ + "The worker id field is required." + ], + "task_queue": [ + "The task queue field is required." + ] + }, + "message": "The worker id field is required. (and 1 more error)", + "protocol_version": "1.20", + "reason": "validation_failed", + "validation_errors": { + "worker_id": [ + "The worker id field is required." + ], + "task_queue": [ + "The task queue field is required." + ] + } + } + }, + "activity_poll_out_of_range": { + "status": 422, + "server_capabilities": true, + "body": { + "errors": { + "timeout_seconds": [ + "The timeout seconds field must be at least 0." + ] + }, + "message": "The timeout seconds field must be at least 0.", + "protocol_version": "1.20", + "reason": "validation_failed", + "validation_errors": { + "timeout_seconds": [ + "The timeout seconds field must be at least 0." + ] + } + } + }, + "activity_complete_unknown": { + "status": 404, + "server_capabilities": true, + "body": { + "activity_attempt_id": "missing-attempt", + "error": "Activity task not found.", + "protocol_version": "1.20", + "reason": "task_not_found", + "task_id": "missing-task" + } + }, + "activity_fail_unknown": { + "status": 404, + "server_capabilities": true, + "body": { + "activity_attempt_id": "missing-attempt", + "error": "Activity task not found.", + "protocol_version": "1.20", + "reason": "task_not_found", + "task_id": "missing-task" + } + }, + "activity_heartbeat_unknown": { + "status": 404, + "server_capabilities": true, + "body": { + "activity_attempt_id": "missing-attempt", + "error": "Activity task not found.", + "protocol_version": "1.20", + "reason": "task_not_found", + "task_id": "missing-task" + } + }, + "route_not_found": { + "status": 404, + "server_capabilities": true, + "body": { + "message": "The route api/worker/nothing-here could not be found.", + "protocol_version": "1.20", + "reason": "not_found" + } + }, + "method_not_allowed": { + "status": 405, + "server_capabilities": true, + "body": { + "message": "The GET method is not supported for route api/worker/workflow-tasks/poll. Supported methods: POST.", + "protocol_version": "1.20", + "reason": "method_not_allowed" + } + }, + "workflow_complete_unknown": { + "status": 404, + "server_capabilities": true, + "body": { + "error": "Workflow task not found.", + "protocol_version": "1.20", + "reason": "task_not_found", + "task_id": "missing-task", + "workflow_task_attempt": 1 + } + } + } + } +} diff --git a/tests/Unit/ServerParityWorkerErrorBodiesContractTest.mjs b/tests/Unit/ServerParityWorkerErrorBodiesContractTest.mjs new file mode 100644 index 000000000..541bc4633 --- /dev/null +++ b/tests/Unit/ServerParityWorkerErrorBodiesContractTest.mjs @@ -0,0 +1,40 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import {readFileSync} from 'node:fs'; +import {checkWorkerErrorBodies} from '../../scripts/conformance/server-parity/worker-error-bodies-contract.mjs'; + +const fixture = JSON.parse(readFileSync(new URL('../Fixtures/ServerParity/worker-error-bodies.json', import.meta.url), 'utf8')); +const expected = fixture.worker_error_bodies.expected; +const observation = () => structuredClone({mode: 'http', worker_error_bodies: {applicable: true, ...expected}}); +const requests = Object.keys(fixture.worker_error_bodies.requests).length; + +test('the published answers pass', () => { + assert.deepStrictEqual(checkWorkerErrorBodies(fixture, observation()), {scope: 'worker_error_bodies', requests}); +}); + +test('embedded applications have no worker protocol', () => { + assert.deepStrictEqual(checkWorkerErrorBodies(fixture, + {mode: 'embedded', worker_error_bodies: {applicable: false, reason: 'embedded_has_no_worker_protocol'}}), + {scope: 'worker_error_bodies', requests}); +}); + +test('every answer is in the worker envelope', () => { + for (const [name, answer] of Object.entries(expected)) { + assert.deepStrictEqual([answer.server_capabilities, answer.body.protocol_version], [true, '1.20'], name); + } +}); + +for (const [name, mutate] of [ + ['a bare invalid_request poll refusal', state => { + state.workflow_poll_empty = {status: 422, server_capabilities: false, body: {message: 'invalid_request', reason: 'invalid_request'}}; + }], + ['an unknown activity failure without its task fields', state => { + state.activity_fail_unknown.body = {message: 'task_not_found', reason: 'task_not_found'}; + state.activity_fail_unknown.server_capabilities = false; + }], + ['a poll refusal without validation_errors', state => { delete state.activity_poll_empty.body.validation_errors; }], + ['an unexpected request', state => { state.extra = {}; }], +]) test(`the contract rejects ${name}`, () => { + const raw = observation(); mutate(raw.worker_error_bodies); + assert.throws(() => checkWorkerErrorBodies(fixture, raw)); +}); From f18176e676b27796a3e4bb2951ac67feb4a58bc2 Mon Sep 17 00:00:00 2001 From: Richard McDaniel <1130888+rmcdaniel@users.noreply.github.com> Date: Thu, 29 Oct 2026 11:39:32 +0000 Subject: [PATCH 2/4] Rust #325: report duplicate poll task kinds with the other failures From #858 review. The poll rules left distinct out of task_kinds.* and let native's task-kinds reader report duplicates afterwards, so a poll that also failed another rule lost PHP's per-index duplicate errors. The Laravel rule engine gains distinct (every duplicate fails, compared by PHP string), and the workflow poll's rules include it. A combined case (timeout_seconds 61 with task_kinds [workflow, workflow]) joins the worker-error-bodies fixture, recorded against published 2.11.5. Co-Authored-By: Claude Opus 5.5 --- rust/src/runtime/http.rs | 5 +-- rust/src/runtime/laravel_rules.rs | 24 +++++++++- rust/tests/execution/worker_error_bodies.rs | 8 ++++ .../ServerParity/worker-error-bodies.json | 44 +++++++++++++++++++ 4 files changed, 76 insertions(+), 5 deletions(-) diff --git a/rust/src/runtime/http.rs b/rust/src/runtime/http.rs index df0a6075a..69c1e1846 100644 --- a/rust/src/runtime/http.rs +++ b/rust/src/runtime/http.rs @@ -2096,8 +2096,7 @@ async fn deregister_fenced( } } } -/// `WorkerController::pollWorkflowTasks`' request rules. Native checks -/// `task_kinds.*` values and their distinctness when it reads them. +/// `WorkerController::pollWorkflowTasks`' request rules. const WORKFLOW_POLL_RULES: &super::laravel_rules::Rules = &[ ("worker_id", &[Rule::Required, Rule::String]), ("task_queue", &[Rule::Required, Rule::String]), @@ -2107,7 +2106,7 @@ const WORKFLOW_POLL_RULES: &super::laravel_rules::Rules = &[ ("history_page_size", &[Rule::Nullable, Rule::Integer, Rule::Min(1), Rule::Max(1000)]), ("accept_history_encoding", &[Rule::Nullable, Rule::String, Rule::Max(64)]), ("task_kinds", &[Rule::Nullable, Rule::Array, Rule::Min(1), Rule::Max(2)]), - ("task_kinds.*", &[Rule::String, Rule::In(&["workflow", "update_validation"])]), + ("task_kinds.*", &[Rule::String, Rule::Distinct, Rule::In(&["workflow", "update_validation"])]), ]; /// `ActivityTaskController::poll`'s request rules. diff --git a/rust/src/runtime/laravel_rules.rs b/rust/src/runtime/laravel_rules.rs index a7c0669a7..49127614d 100644 --- a/rust/src/runtime/laravel_rules.rs +++ b/rust/src/runtime/laravel_rules.rs @@ -47,6 +47,9 @@ pub(crate) enum Rule { Regex(fn(&str) -> bool), /// `prohibited`: the attribute is missing, null or empty. Implicit. Prohibited, + /// `distinct` on a `x.*` attribute: no other item of the same array + /// holds the same value. Every duplicate fails, the first included. + Distinct, } pub(crate) type Rules = [(&'static str, &'static [Rule])]; @@ -226,6 +229,7 @@ fn message(rule: Rule, attribute: &str, rules: &[Rule], value: Option<&Value>) - }, Rule::NotBlank | Rule::Pattern(_) | Rule::Regex(_) => format!("The {attribute} field format is invalid."), Rule::Prohibited => format!("The {attribute} field is prohibited."), + Rule::Distinct => format!("The {attribute} field has a duplicate value."), Rule::Date => format!("The {attribute} field must be a valid date."), Rule::Nullable | Rule::Sometimes => String::new(), } @@ -268,7 +272,7 @@ fn passes(rule: Rule, data: &Value, value: Option<&Value>, rules: &[Rule]) -> bo Rule::Required => filled(value), Rule::Present => value.is_some(), Rule::RequiredWith(other) => !filled(lookup(data, other)) || filled(value), - Rule::Nullable | Rule::Sometimes => true, + Rule::Nullable | Rule::Sometimes | Rule::Distinct => true, Rule::NotBlank => value.and_then(Value::as_str).is_some_and(|text| text.chars().any(|character| !character.is_whitespace())), Rule::String => value.is_some_and(Value::is_string), Rule::Pattern(matches) => value.and_then(Value::as_str).is_some_and(matches), @@ -349,6 +353,21 @@ fn checks(body: &Value, rules: &Rules) -> Vec<(String, bool, &'static [Rule])> { /// One attribute's messages. An implicit rule (required, required_with, /// present) stops at its first failure, as Laravel's does. +/// Laravel's `validateDistinct` for an expanded `x.N` attribute: another +/// item of `x` has the same value, compared as PHP compares them loosely +/// for scalars (their string forms). +fn duplicated(body: &Value, attribute: &str, value: Option<&Value>) -> bool { + let Some((parent, index)) = attribute.rsplit_once('.') else { + return false; + }; + let (Some(Value::Array(items)), Some(own)) = (lookup(body, parent), value.and_then(php_string)) else { + return false; + }; + items.iter().enumerate().any(|(position, item)| { + position.to_string() != index && php_string(item).is_some_and(|other| other == own) + }) +} + fn attribute_errors(body: &Value, attribute: &str, expanded: bool, list: &[Rule]) -> Vec { let value = lookup(body, attribute); if value.is_none() && list.iter().any(|rule| matches!(rule, Rule::Sometimes)) { @@ -365,7 +384,8 @@ fn attribute_errors(body: &Value, attribute: &str, expanded: bool, list: &[Rule] if !implicit && skipped(value, nullable) { continue; } - if !passes(*rule, body, value, list) { + let failed = if matches!(rule, Rule::Distinct) { duplicated(body, attribute, value) } else { !passes(*rule, body, value, list) }; + if failed { messages.push(message(*rule, &name, list, value)); if implicit { break; diff --git a/rust/tests/execution/worker_error_bodies.rs b/rust/tests/execution/worker_error_bodies.rs index 23cbf1e7f..1437f9f6e 100644 --- a/rust/tests/execution/worker_error_bodies.rs +++ b/rust/tests/execution/worker_error_bodies.rs @@ -24,6 +24,14 @@ async fn poll_validation_failures_carry_laravels_messages_in_the_envelope() { assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY, "{body}"); assert_eq!(body["message"], "The poll request id field must not be greater than 255 characters. (and 3 more errors)"); assert_eq!(body["errors"]["task_kinds"], json!(["The task kinds field must have at least 1 items."])); + // `distinct` is part of the same validation: duplicate kinds are reported + // beside the other fields' failures, every duplicate index included. + let (status, body) = request_protocol(&app, "POST", "/api/worker/workflow-tasks/poll", json!({"worker_id":"w","task_queue":"q", + "timeout_seconds":61,"task_kinds":["workflow","workflow"]}), "1.20").await; + assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY, "{body}"); + assert_eq!(body["errors"], json!({"timeout_seconds":["The timeout seconds field must not be greater than 60."], + "task_kinds.0":["The task_kinds.0 field has a duplicate value."],"task_kinds.1":["The task_kinds.1 field has a duplicate value."]})); + assert_eq!(body["message"], "The timeout seconds field must not be greater than 60. (and 2 more errors)"); let (status, body) = request_protocol(&app, "POST", "/api/worker/activity-tasks/poll", json!({"worker_id":"w","task_queue":"q","timeout_seconds":-1}), "1.20").await; assert_eq!((status, body["errors"].clone()), (StatusCode::UNPROCESSABLE_ENTITY, diff --git a/tests/Fixtures/ServerParity/worker-error-bodies.json b/tests/Fixtures/ServerParity/worker-error-bodies.json index 650bcd271..4f212d835 100644 --- a/tests/Fixtures/ServerParity/worker-error-bodies.json +++ b/tests/Fixtures/ServerParity/worker-error-bodies.json @@ -118,6 +118,19 @@ } ] } + ], + "workflow_poll_duplicate_kinds": [ + "POST", + "worker/workflow-tasks/poll", + { + "worker_id": "w", + "task_queue": "q", + "timeout_seconds": 61, + "task_kinds": [ + "workflow", + "workflow" + ] + } ] }, "expected": { @@ -288,6 +301,37 @@ "task_id": "missing-task", "workflow_task_attempt": 1 } + }, + "workflow_poll_duplicate_kinds": { + "status": 422, + "server_capabilities": true, + "body": { + "errors": { + "timeout_seconds": [ + "The timeout seconds field must not be greater than 60." + ], + "task_kinds.0": [ + "The task_kinds.0 field has a duplicate value." + ], + "task_kinds.1": [ + "The task_kinds.1 field has a duplicate value." + ] + }, + "message": "The timeout seconds field must not be greater than 60. (and 2 more errors)", + "protocol_version": "1.20", + "reason": "validation_failed", + "validation_errors": { + "timeout_seconds": [ + "The timeout seconds field must not be greater than 60." + ], + "task_kinds.0": [ + "The task_kinds.0 field has a duplicate value." + ], + "task_kinds.1": [ + "The task_kinds.1 field has a duplicate value." + ] + } + } } } } From df58e41056607cb1e146e10c112bf8b49d13839e Mon Sep 17 00:00:00 2001 From: Richard McDaniel <1130888+rmcdaniel@users.noreply.github.com> Date: Thu, 29 Oct 2026 12:41:11 +0000 Subject: [PATCH 3/4] Rust #325: compare distinct values with PHP's loose equality From #858 review. Laravel's unqualified distinct finds duplicates with in_array's loose ==, so true equals "workflow"; native compared PHP string casts and missed it. php_value::loose_eq implements PHP 8's == for decoded JSON values (booleans by truthiness, null against empty and zero values, numbers and numeric strings numerically, other strings exactly, arrays key by key), with a unit test, and distinct uses it. The worker-error-bodies fixture gains a poll with task_kinds [workflow, true], recorded against published 2.11.5. Co-Authored-By: Claude Opus 5.5 --- rust/src/runtime/laravel_rules.rs | 8 +-- rust/src/runtime/php_value.rs | 70 +++++++++++++++++++ rust/tests/execution/worker_error_bodies.rs | 7 ++ .../ServerParity/worker-error-bodies.json | 41 +++++++++++ 4 files changed, 122 insertions(+), 4 deletions(-) diff --git a/rust/src/runtime/laravel_rules.rs b/rust/src/runtime/laravel_rules.rs index 49127614d..40a80d941 100644 --- a/rust/src/runtime/laravel_rules.rs +++ b/rust/src/runtime/laravel_rules.rs @@ -354,17 +354,17 @@ fn checks(body: &Value, rules: &Rules) -> Vec<(String, bool, &'static [Rule])> { /// One attribute's messages. An implicit rule (required, required_with, /// present) stops at its first failure, as Laravel's does. /// Laravel's `validateDistinct` for an expanded `x.N` attribute: another -/// item of `x` has the same value, compared as PHP compares them loosely -/// for scalars (their string forms). +/// item of `x` equals this one under PHP's loose `==`, as the unqualified +/// rule's `in_array` compares them. fn duplicated(body: &Value, attribute: &str, value: Option<&Value>) -> bool { let Some((parent, index)) = attribute.rsplit_once('.') else { return false; }; - let (Some(Value::Array(items)), Some(own)) = (lookup(body, parent), value.and_then(php_string)) else { + let (Some(Value::Array(items)), Some(own)) = (lookup(body, parent), value) else { return false; }; items.iter().enumerate().any(|(position, item)| { - position.to_string() != index && php_string(item).is_some_and(|other| other == own) + position.to_string() != index && super::php_value::loose_eq(own, item) }) } diff --git a/rust/src/runtime/php_value.rs b/rust/src/runtime/php_value.rs index 6301e4af0..efe6cae68 100644 --- a/rust/src/runtime/php_value.rs +++ b/rust/src/runtime/php_value.rs @@ -578,6 +578,59 @@ pub(super) fn numeric(text: &str) -> Option { trimmed.parse::().ok() } +/// PHP 8's `==` between two decoded JSON values, as `in_array` uses it by +/// default: a boolean compares by truthiness, null equals the empty and the +/// zero values, numbers and numeric strings compare as numbers, other +/// strings compare exactly, and arrays compare key by key. +pub(crate) fn loose_eq(left: &serde_json::Value, right: &serde_json::Value) -> bool { + use serde_json::Value; + fn truthy(value: &Value) -> bool { + match value { + Value::Null => false, + Value::Bool(flag) => *flag, + Value::Number(number) => number.as_f64().is_some_and(|number| number != 0.0), + Value::String(text) => !text.is_empty() && text != "0", + Value::Array(items) => !items.is_empty(), + Value::Object(fields) => !fields.is_empty(), + } + } + fn number_text(number: &serde_json::Number) -> String { + match number.as_i64() { + Some(whole) => whole.to_string(), + None => float(number.as_f64().unwrap_or_default(), false), + } + } + fn entries(value: &Value) -> Option> { + match value { + Value::Array(items) => Some(items.iter().enumerate().map(|(index, item)| (index.to_string(), item)).collect()), + Value::Object(fields) => Some(fields.iter().map(|(key, item)| (key.clone(), item)).collect()), + _ => None, + } + } + match (left, right) { + (Value::Bool(flag), other) | (other, Value::Bool(flag)) => *flag == truthy(other), + (Value::Null, other) | (other, Value::Null) => !truthy(other) && !matches!(other, Value::String(text) if text == "0"), + (Value::Number(a), Value::Number(b)) => match (a.as_i64(), b.as_i64()) { + (Some(a), Some(b)) => a == b, + _ => a.as_f64() == b.as_f64(), + }, + (Value::Number(number), Value::String(text)) | (Value::String(text), Value::Number(number)) => match numeric(text) { + Some(parsed) => number.as_f64() == Some(parsed), + None => number_text(number) == *text, + }, + (Value::String(a), Value::String(b)) => match (numeric(a), numeric(b)) { + (Some(a), Some(b)) => a == b, + _ => a == b, + }, + (Value::Array(_) | Value::Object(_), Value::Array(_) | Value::Object(_)) => { + let (a, b) = (entries(left).unwrap_or_default(), entries(right).unwrap_or_default()); + a.len() == b.len() + && a.iter().all(|(key, item)| b.iter().any(|(other, value)| other == key && loose_eq(item, value))) + } + _ => false, + } +} + /// PHP's `(int)` cast of a numeric string. pub(super) fn int_cast(text: &str) -> Option { let value = numeric(text)?; @@ -602,6 +655,23 @@ pub(super) fn int_cast(text: &str) -> Option { #[cfg(test)] mod tests { + #[test] + fn loose_eq_follows_php_8() { + use serde_json::json; + for (a, b, equal) in [ + (json!(true), json!("workflow"), true), (json!(true), json!(""), false), (json!(false), json!("0"), true), + (json!(null), json!(""), true), (json!(null), json!(0), true), (json!(null), json!("0"), false), (json!(null), json!([]), true), + (json!(1), json!("1"), true), (json!(1), json!("01"), true), (json!(1), json!("1e0"), true), (json!(0), json!("a"), false), + (json!("1"), json!("01"), true), (json!("abc"), json!("ABC"), false), (json!("1e3"), json!("1000"), true), + (json!(1.5), json!(1.5), true), (json!(1), json!(1.0), true), + (json!([1, 2]), json!(["1", "2"]), true), (json!([1, 2]), json!([2, 1]), false), (json!({"a": 1}), json!({"a": "1"}), true), + (json!("workflow"), json!("update_validation"), false), (json!([]), json!(0), false), + ] { + assert_eq!(loose_eq(&a, &b), equal, "{a} == {b}"); + assert_eq!(loose_eq(&b, &a), equal, "{b} == {a}"); + } + } + use super::*; fn decode(text: &str) -> Php { diff --git a/rust/tests/execution/worker_error_bodies.rs b/rust/tests/execution/worker_error_bodies.rs index 1437f9f6e..b2c08a214 100644 --- a/rust/tests/execution/worker_error_bodies.rs +++ b/rust/tests/execution/worker_error_bodies.rs @@ -32,6 +32,13 @@ async fn poll_validation_failures_carry_laravels_messages_in_the_envelope() { assert_eq!(body["errors"], json!({"timeout_seconds":["The timeout seconds field must not be greater than 60."], "task_kinds.0":["The task_kinds.0 field has a duplicate value."],"task_kinds.1":["The task_kinds.1 field has a duplicate value."]})); assert_eq!(body["message"], "The timeout seconds field must not be greater than 60. (and 2 more errors)"); + // `distinct` compares with PHP's loose `==`: true equals "workflow". + let (status, body) = request_protocol(&app, "POST", "/api/worker/workflow-tasks/poll", + json!({"worker_id":"w","task_queue":"q","task_kinds":["workflow",true]}), "1.20").await; + assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY, "{body}"); + assert_eq!(body["errors"], json!({"task_kinds.0":["The task_kinds.0 field has a duplicate value."], + "task_kinds.1":["The task_kinds.1 field must be a string.","The task_kinds.1 field has a duplicate value.", + "The selected task_kinds.1 is invalid."]})); let (status, body) = request_protocol(&app, "POST", "/api/worker/activity-tasks/poll", json!({"worker_id":"w","task_queue":"q","timeout_seconds":-1}), "1.20").await; assert_eq!((status, body["errors"].clone()), (StatusCode::UNPROCESSABLE_ENTITY, diff --git a/tests/Fixtures/ServerParity/worker-error-bodies.json b/tests/Fixtures/ServerParity/worker-error-bodies.json index 4f212d835..4e45c881f 100644 --- a/tests/Fixtures/ServerParity/worker-error-bodies.json +++ b/tests/Fixtures/ServerParity/worker-error-bodies.json @@ -131,6 +131,18 @@ "workflow" ] } + ], + "workflow_poll_loose_duplicate_kinds": [ + "POST", + "worker/workflow-tasks/poll", + { + "worker_id": "w", + "task_queue": "q", + "task_kinds": [ + "workflow", + true + ] + } ] }, "expected": { @@ -332,6 +344,35 @@ ] } } + }, + "workflow_poll_loose_duplicate_kinds": { + "status": 422, + "server_capabilities": true, + "body": { + "errors": { + "task_kinds.0": [ + "The task_kinds.0 field has a duplicate value." + ], + "task_kinds.1": [ + "The task_kinds.1 field must be a string.", + "The task_kinds.1 field has a duplicate value.", + "The selected task_kinds.1 is invalid." + ] + }, + "message": "The task_kinds.0 field has a duplicate value. (and 3 more errors)", + "protocol_version": "1.20", + "reason": "validation_failed", + "validation_errors": { + "task_kinds.0": [ + "The task_kinds.0 field has a duplicate value." + ], + "task_kinds.1": [ + "The task_kinds.1 field must be a string.", + "The task_kinds.1 field has a duplicate value.", + "The selected task_kinds.1 is invalid." + ] + } + } } } } From 609fde249ef87b397a3679339d8c6541a5784e27 Mon Sep 17 00:00:00 2001 From: Richard McDaniel <1130888+rmcdaniel@users.noreply.github.com> Date: Thu, 29 Oct 2026 13:34:18 +0000 Subject: [PATCH 4/4] Rust #325: compare numeric strings and distinct candidates as Laravel does From #858 review. - loose_eq compared numeric strings as floats, so "9007199254740992" equalled "9007199254740993". It now follows PHP 8: integer strings that fit compare as integers, an integer against a numeric string as compare_longs_to_string does, and two numeric strings as zendi_smart_str_equals does (integers overflowing to the same side, or equal infinities, compare as text). - distinct took candidates only from a list parent. It now takes them as Laravel's getDistinctValues does through Arr::dot: list or associative items, scalars, null and empty arrays, but not non-empty nested arrays. Eight control requests join the worker-error-bodies fixture, recorded against published 2.11.5, and a native test replays every fixture request and compares PHP's recorded answers. Co-Authored-By: Claude Opus 5.5 --- .../2026-10-29-worker-error-bodies.md | 12 +- rust/src/runtime/laravel_rules.rs | 26 +- rust/src/runtime/php_value.rs | 81 ++++- rust/tests/execution/worker_error_bodies.rs | 23 ++ .../ServerParity/worker-error-bodies.json | 339 ++++++++++++++++++ 5 files changed, 456 insertions(+), 25 deletions(-) diff --git a/docs/rust-server-port-checkpoints/2026-10-29-worker-error-bodies.md b/docs/rust-server-port-checkpoints/2026-10-29-worker-error-bodies.md index b93232156..18ede4361 100644 --- a/docs/rust-server-port-checkpoints/2026-10-29-worker-error-bodies.md +++ b/docs/rust-server-port-checkpoints/2026-10-29-worker-error-bodies.md @@ -33,7 +33,13 @@ with `task_id`, `workflow_task_attempt` and "Workflow task not found." The ownership guard runs before the command normalizer reads payloads. - **Fixture.** `worker-error-bodies`, raw worker HTTP on protocol 1.20 in a - fresh namespace: ten requests. The probe records each status and body, + fresh namespace: twenty requests. Ten of them probe `distinct` on + `task_kinds.*` (from review). It runs in the same pass as the other rules, + and compares candidates as Laravel takes them (`Arr::dot`: list or + associative items, scalars and empty arrays, not non-empty nested arrays) + with PHP 8's loose `==`. That includes `zendi_smart_str_equals` for numeric + strings: integers that fit compare exactly, and integers overflowing to the + same side, or equal infinities, compare as text. The probe records each status and body, with `server_capabilities` reduced to whether it is present. - **Native.** `http.rs` validates both polls with the published rules before reading them (native's task-kinds reader still checks distinct values), @@ -45,4 +51,6 @@ are still refused that way. A workflow task completion or checkpoint now looks the task up before checking or fetching any command payload. - **Tests.** `worker_error_bodies.rs` fails on the previous source and - passes now. + passes now. Its last test replays every fixture request against native + and compares PHP's recorded answers. `php_value::loose_eq` has its own + unit test. diff --git a/rust/src/runtime/laravel_rules.rs b/rust/src/runtime/laravel_rules.rs index 40a80d941..6d8ed35b5 100644 --- a/rust/src/runtime/laravel_rules.rs +++ b/rust/src/runtime/laravel_rules.rs @@ -353,18 +353,30 @@ fn checks(body: &Value, rules: &Rules) -> Vec<(String, bool, &'static [Rule])> { /// One attribute's messages. An implicit rule (required, required_with, /// present) stops at its first failure, as Laravel's does. -/// Laravel's `validateDistinct` for an expanded `x.N` attribute: another -/// item of `x` equals this one under PHP's loose `==`, as the unqualified -/// rule's `in_array` compares them. +/// Laravel's `validateDistinct` for an expanded `x.K` attribute. The +/// candidates are `getDistinctValues`: the other items of `x`, list or +/// associative, that `Arr::dot` leaves at that depth (scalars, null and +/// empty arrays; a non-empty nested array is flattened past it). This +/// item is a duplicate when one of them equals it under PHP's loose `==`. fn duplicated(body: &Value, attribute: &str, value: Option<&Value>) -> bool { - let Some((parent, index)) = attribute.rsplit_once('.') else { + let Some((parent, key)) = attribute.rsplit_once('.') else { return false; }; - let (Some(Value::Array(items)), Some(own)) = (lookup(body, parent), value) else { + let Some(own) = value else { return false; }; - items.iter().enumerate().any(|(position, item)| { - position.to_string() != index && super::php_value::loose_eq(own, item) + let siblings: Vec<(String, &Value)> = match lookup(body, parent) { + Some(Value::Array(items)) => items.iter().enumerate().map(|(index, item)| (index.to_string(), item)).collect(), + Some(Value::Object(fields)) => fields.iter().map(|(name, item)| (name.clone(), item)).collect(), + _ => return false, + }; + siblings.into_iter().any(|(other, item)| { + let flattened = match item { + Value::Array(items) => !items.is_empty(), + Value::Object(fields) => !fields.is_empty(), + _ => false, + }; + other != key && !flattened && super::php_value::loose_eq(own, item) }) } diff --git a/rust/src/runtime/php_value.rs b/rust/src/runtime/php_value.rs index efe6cae68..aeed9c27f 100644 --- a/rust/src/runtime/php_value.rs +++ b/rust/src/runtime/php_value.rs @@ -578,10 +578,54 @@ pub(super) fn numeric(text: &str) -> Option { trimmed.parse::().ok() } +/// A numeric string as PHP's `is_numeric_string_ex` reads it: an integer +/// that fits, or a double, with `oflow` set (+1 or -1) for an integer-syntax +/// string beyond the 64-bit range. +#[derive(Clone, Copy)] +enum NumericString { + Long(i64), + Double { value: f64, oflow: i8 }, +} + +fn numeric_string(text: &str) -> Option { + let value = numeric(text)?; + let trimmed = text.trim_matches([' ', '\t', '\n', '\r', '\x0b', '\x0c']); + let integer_syntax = !trimmed.contains(['.', 'e', 'E']); + if integer_syntax { + if let Ok(whole) = trimmed.parse::() { + return Some(NumericString::Long(whole)); + } + return Some(NumericString::Double { value, oflow: if trimmed.starts_with('-') { -1 } else { 1 } }); + } + Some(NumericString::Double { value, oflow: 0 }) +} + +/// PHP 8's `zendi_smart_str_equals`: numeric strings compare as numbers, +/// except that two integers overflowing to the same side, or two equal +/// infinities, compare as text; any other pair compares as text. +fn smart_str_equals(left: &str, right: &str) -> bool { + let (Some(a), Some(b)) = (numeric_string(left), numeric_string(right)) else { + return left == right; + }; + match (a, b) { + (NumericString::Long(a), NumericString::Long(b)) => a == b, + (NumericString::Double { value: a, oflow: oa }, NumericString::Double { value: b, oflow: ob }) => { + if (oa != 0 && oa == ob && a - b == 0.0) || (a == b && !a.is_finite()) { + left == right + } else { + a == b + } + } + (NumericString::Long(whole), NumericString::Double { value, oflow }) + | (NumericString::Double { value, oflow }, NumericString::Long(whole)) => oflow == 0 && whole as f64 == value, + } +} + /// PHP 8's `==` between two decoded JSON values, as `in_array` uses it by /// default: a boolean compares by truthiness, null equals the empty and the -/// zero values, numbers and numeric strings compare as numbers, other -/// strings compare exactly, and arrays compare key by key. +/// zero values, an integer against a numeric string compares as integers +/// when the string is one, numeric strings compare as `smart_str_equals` +/// does, other strings compare exactly, and arrays compare key by key. pub(crate) fn loose_eq(left: &serde_json::Value, right: &serde_json::Value) -> bool { use serde_json::Value; fn truthy(value: &Value) -> bool { @@ -594,12 +638,6 @@ pub(crate) fn loose_eq(left: &serde_json::Value, right: &serde_json::Value) -> b Value::Object(fields) => !fields.is_empty(), } } - fn number_text(number: &serde_json::Number) -> String { - match number.as_i64() { - Some(whole) => whole.to_string(), - None => float(number.as_f64().unwrap_or_default(), false), - } - } fn entries(value: &Value) -> Option> { match value { Value::Array(items) => Some(items.iter().enumerate().map(|(index, item)| (index.to_string(), item)).collect()), @@ -614,14 +652,20 @@ pub(crate) fn loose_eq(left: &serde_json::Value, right: &serde_json::Value) -> b (Some(a), Some(b)) => a == b, _ => a.as_f64() == b.as_f64(), }, - (Value::Number(number), Value::String(text)) | (Value::String(text), Value::Number(number)) => match numeric(text) { - Some(parsed) => number.as_f64() == Some(parsed), - None => number_text(number) == *text, - }, - (Value::String(a), Value::String(b)) => match (numeric(a), numeric(b)) { - (Some(a), Some(b)) => a == b, - _ => a == b, - }, + // `compare_longs_to_string` and `compare_doubles_to_string`. + (Value::Number(number), Value::String(text)) | (Value::String(text), Value::Number(number)) => { + match (number.as_i64(), numeric_string(text)) { + (Some(whole), Some(NumericString::Long(other))) => whole == other, + (Some(whole), Some(NumericString::Double { value, .. })) => whole as f64 == value, + (None, Some(NumericString::Long(other))) => number.as_f64() == Some(other as f64), + (None, Some(NumericString::Double { value, .. })) => number.as_f64() == Some(value), + (_, None) => match number.as_i64() { + Some(whole) => whole.to_string() == *text, + None => float(number.as_f64().unwrap_or_default(), false) == *text, + }, + } + } + (Value::String(a), Value::String(b)) => smart_str_equals(a, b), (Value::Array(_) | Value::Object(_), Value::Array(_) | Value::Object(_)) => { let (a, b) = (entries(left).unwrap_or_default(), entries(right).unwrap_or_default()); a.len() == b.len() @@ -666,6 +710,11 @@ mod tests { (json!(1.5), json!(1.5), true), (json!(1), json!(1.0), true), (json!([1, 2]), json!(["1", "2"]), true), (json!([1, 2]), json!([2, 1]), false), (json!({"a": 1}), json!({"a": "1"}), true), (json!("workflow"), json!("update_validation"), false), (json!([]), json!(0), false), + (json!("9007199254740992"), json!("9007199254740993"), false), (json!(9007199254740992_i64), json!("9007199254740993"), false), + (json!("9223372036854775808"), json!("9223372036854775809"), false), + (json!("18446744073709551615"), json!("18446744073709551616"), false), (json!("1e9999"), json!("2e9999"), false), + (json!("9007199254740992e0"), json!("9007199254740993e0"), true), (json!("9223372036854775808"), json!("9223372036854775808"), true), + (json!("10"), json!("1e1"), true), (json!(" 1"), json!("1"), true), ] { assert_eq!(loose_eq(&a, &b), equal, "{a} == {b}"); assert_eq!(loose_eq(&b, &a), equal, "{b} == {a}"); diff --git a/rust/tests/execution/worker_error_bodies.rs b/rust/tests/execution/worker_error_bodies.rs index b2c08a214..a07a385da 100644 --- a/rust/tests/execution/worker_error_bodies.rs +++ b/rust/tests/execution/worker_error_bodies.rs @@ -112,3 +112,26 @@ async fn an_unknown_workflow_task_is_answered_before_its_payloads_are_read() { runtime.close().await; database.remove().await; } + +// Every request of the worker-error-bodies fixture, answered as published +// 2.11.5 answered it there (`server_capabilities` reduced to its presence). +#[tokio::test] +async fn the_fixture_requests_are_answered_as_published_php_answered_them() { + let fixture: Value = serde_json::from_str(include_str!("../../../tests/Fixtures/ServerParity/worker-error-bodies.json")).unwrap(); + let definition = &fixture["worker_error_bodies"]; + let database = TestDatabase::new().await; + let runtime = database.open().await.unwrap(); + let app = router(runtime.clone()); + for (name, request) in definition["requests"].as_object().unwrap() { + let (method, path, body) = (request[0].as_str().unwrap(), request[1].as_str().unwrap(), request[2].clone()); + let (status, mut answer) = request_protocol(&app, method, &format!("/api/{path}"), body, "1.20").await; + let capabilities = answer["server_capabilities"].is_object(); + answer.as_object_mut().unwrap().remove("server_capabilities"); + let expected = &definition["expected"][name]; + assert_eq!((status.as_u16(), capabilities), (expected["status"].as_u64().unwrap() as u16, expected["server_capabilities"] == true), + "{name}: {answer}"); + assert_eq!(answer, expected["body"], "{name}"); + } + runtime.close().await; + database.remove().await; +} diff --git a/tests/Fixtures/ServerParity/worker-error-bodies.json b/tests/Fixtures/ServerParity/worker-error-bodies.json index 4e45c881f..84f675061 100644 --- a/tests/Fixtures/ServerParity/worker-error-bodies.json +++ b/tests/Fixtures/ServerParity/worker-error-bodies.json @@ -143,6 +143,109 @@ true ] } + ], + "workflow_poll_associative_kinds": [ + "POST", + "worker/workflow-tasks/poll", + { + "worker_id": "w", + "task_queue": "q", + "timeout_seconds": 61, + "task_kinds": { + "a": "workflow", + "b": "workflow" + } + } + ], + "workflow_poll_nested_kinds": [ + "POST", + "worker/workflow-tasks/poll", + { + "worker_id": "w", + "task_queue": "q", + "timeout_seconds": 61, + "task_kinds": [ + [ + "workflow" + ], + [ + "workflow" + ] + ] + } + ], + "workflow_poll_empty_nested_kinds": [ + "POST", + "worker/workflow-tasks/poll", + { + "worker_id": "w", + "task_queue": "q", + "timeout_seconds": 61, + "task_kinds": [ + [], + [] + ] + } + ], + "workflow_poll_large_integer_kinds": [ + "POST", + "worker/workflow-tasks/poll", + { + "worker_id": "w", + "task_queue": "q", + "task_kinds": [ + "9007199254740992", + "9007199254740993" + ] + } + ], + "workflow_poll_integer_string_kinds": [ + "POST", + "worker/workflow-tasks/poll", + { + "worker_id": "w", + "task_queue": "q", + "task_kinds": [ + 9007199254740992, + "9007199254740993" + ] + } + ], + "workflow_poll_overflow_kinds": [ + "POST", + "worker/workflow-tasks/poll", + { + "worker_id": "w", + "task_queue": "q", + "task_kinds": [ + "9223372036854775808", + "9223372036854775809" + ] + } + ], + "workflow_poll_infinite_kinds": [ + "POST", + "worker/workflow-tasks/poll", + { + "worker_id": "w", + "task_queue": "q", + "task_kinds": [ + "1e9999", + "2e9999" + ] + } + ], + "workflow_poll_rounded_kinds": [ + "POST", + "worker/workflow-tasks/poll", + { + "worker_id": "w", + "task_queue": "q", + "task_kinds": [ + "9007199254740992e0", + "9007199254740993e0" + ] + } ] }, "expected": { @@ -373,6 +476,242 @@ ] } } + }, + "workflow_poll_associative_kinds": { + "status": 422, + "server_capabilities": true, + "body": { + "errors": { + "timeout_seconds": [ + "The timeout seconds field must not be greater than 60." + ], + "task_kinds.a": [ + "The task_kinds.a field has a duplicate value." + ], + "task_kinds.b": [ + "The task_kinds.b field has a duplicate value." + ] + }, + "message": "The timeout seconds field must not be greater than 60. (and 2 more errors)", + "protocol_version": "1.20", + "reason": "validation_failed", + "validation_errors": { + "timeout_seconds": [ + "The timeout seconds field must not be greater than 60." + ], + "task_kinds.a": [ + "The task_kinds.a field has a duplicate value." + ], + "task_kinds.b": [ + "The task_kinds.b field has a duplicate value." + ] + } + } + }, + "workflow_poll_nested_kinds": { + "status": 422, + "server_capabilities": true, + "body": { + "errors": { + "timeout_seconds": [ + "The timeout seconds field must not be greater than 60." + ], + "task_kinds.0": [ + "The task_kinds.0 field must be a string.", + "The selected task_kinds.0 is invalid." + ], + "task_kinds.1": [ + "The task_kinds.1 field must be a string.", + "The selected task_kinds.1 is invalid." + ] + }, + "message": "The timeout seconds field must not be greater than 60. (and 4 more errors)", + "protocol_version": "1.20", + "reason": "validation_failed", + "validation_errors": { + "timeout_seconds": [ + "The timeout seconds field must not be greater than 60." + ], + "task_kinds.0": [ + "The task_kinds.0 field must be a string.", + "The selected task_kinds.0 is invalid." + ], + "task_kinds.1": [ + "The task_kinds.1 field must be a string.", + "The selected task_kinds.1 is invalid." + ] + } + } + }, + "workflow_poll_empty_nested_kinds": { + "status": 422, + "server_capabilities": true, + "body": { + "errors": { + "timeout_seconds": [ + "The timeout seconds field must not be greater than 60." + ], + "task_kinds.0": [ + "The task_kinds.0 field must be a string.", + "The task_kinds.0 field has a duplicate value.", + "The selected task_kinds.0 is invalid." + ], + "task_kinds.1": [ + "The task_kinds.1 field must be a string.", + "The task_kinds.1 field has a duplicate value.", + "The selected task_kinds.1 is invalid." + ] + }, + "message": "The timeout seconds field must not be greater than 60. (and 6 more errors)", + "protocol_version": "1.20", + "reason": "validation_failed", + "validation_errors": { + "timeout_seconds": [ + "The timeout seconds field must not be greater than 60." + ], + "task_kinds.0": [ + "The task_kinds.0 field must be a string.", + "The task_kinds.0 field has a duplicate value.", + "The selected task_kinds.0 is invalid." + ], + "task_kinds.1": [ + "The task_kinds.1 field must be a string.", + "The task_kinds.1 field has a duplicate value.", + "The selected task_kinds.1 is invalid." + ] + } + } + }, + "workflow_poll_large_integer_kinds": { + "status": 422, + "server_capabilities": true, + "body": { + "errors": { + "task_kinds.0": [ + "The selected task_kinds.0 is invalid." + ], + "task_kinds.1": [ + "The selected task_kinds.1 is invalid." + ] + }, + "message": "The selected task_kinds.0 is invalid. (and 1 more error)", + "protocol_version": "1.20", + "reason": "validation_failed", + "validation_errors": { + "task_kinds.0": [ + "The selected task_kinds.0 is invalid." + ], + "task_kinds.1": [ + "The selected task_kinds.1 is invalid." + ] + } + } + }, + "workflow_poll_integer_string_kinds": { + "status": 422, + "server_capabilities": true, + "body": { + "errors": { + "task_kinds.0": [ + "The task_kinds.0 field must be a string.", + "The selected task_kinds.0 is invalid." + ], + "task_kinds.1": [ + "The selected task_kinds.1 is invalid." + ] + }, + "message": "The task_kinds.0 field must be a string. (and 2 more errors)", + "protocol_version": "1.20", + "reason": "validation_failed", + "validation_errors": { + "task_kinds.0": [ + "The task_kinds.0 field must be a string.", + "The selected task_kinds.0 is invalid." + ], + "task_kinds.1": [ + "The selected task_kinds.1 is invalid." + ] + } + } + }, + "workflow_poll_overflow_kinds": { + "status": 422, + "server_capabilities": true, + "body": { + "errors": { + "task_kinds.0": [ + "The selected task_kinds.0 is invalid." + ], + "task_kinds.1": [ + "The selected task_kinds.1 is invalid." + ] + }, + "message": "The selected task_kinds.0 is invalid. (and 1 more error)", + "protocol_version": "1.20", + "reason": "validation_failed", + "validation_errors": { + "task_kinds.0": [ + "The selected task_kinds.0 is invalid." + ], + "task_kinds.1": [ + "The selected task_kinds.1 is invalid." + ] + } + } + }, + "workflow_poll_infinite_kinds": { + "status": 422, + "server_capabilities": true, + "body": { + "errors": { + "task_kinds.0": [ + "The selected task_kinds.0 is invalid." + ], + "task_kinds.1": [ + "The selected task_kinds.1 is invalid." + ] + }, + "message": "The selected task_kinds.0 is invalid. (and 1 more error)", + "protocol_version": "1.20", + "reason": "validation_failed", + "validation_errors": { + "task_kinds.0": [ + "The selected task_kinds.0 is invalid." + ], + "task_kinds.1": [ + "The selected task_kinds.1 is invalid." + ] + } + } + }, + "workflow_poll_rounded_kinds": { + "status": 422, + "server_capabilities": true, + "body": { + "errors": { + "task_kinds.0": [ + "The task_kinds.0 field has a duplicate value.", + "The selected task_kinds.0 is invalid." + ], + "task_kinds.1": [ + "The task_kinds.1 field has a duplicate value.", + "The selected task_kinds.1 is invalid." + ] + }, + "message": "The task_kinds.0 field has a duplicate value. (and 3 more errors)", + "protocol_version": "1.20", + "reason": "validation_failed", + "validation_errors": { + "task_kinds.0": [ + "The task_kinds.0 field has a duplicate value.", + "The selected task_kinds.0 is invalid." + ], + "task_kinds.1": [ + "The task_kinds.1 field has a duplicate value.", + "The selected task_kinds.1 is invalid." + ] + } + } } } }