ITADN

Fix silent error swallowing and mis-propagation across zsa packages

#248Pull Requeststaging-devin-ai-integration[bot] 创建于 2026-04-20
## Summary Audit + fix pass over error handling in the zsa monorepo. Several paths were catching errors and returning `undefined` / `null` / a mis-shaped default without any logging or propagation, making real failures invisible and — in a few cases — actively misleading. All 130 existing tests still pass; 6 new tests cover the fixes. ### Changes **`zsa-react` — unhandled `serverAction` rejection (<ref_snippet file="packages/zsa-react/src/index.ts" lines="131-165" />)** `await serverAction(...)` used `.then(...)` with no `.catch`. If the call rejected for anything other than `NEXT_REDIRECT` / `NEXT_NOT_FOUND` (network error, RSC transport error, serialization failure) the rejection became an unhandled promise rejection AND `executeRef.current` never resolved — callers who did `await execute()` hung forever. Now we `try/catch`, rethrow Next.js redirect/notFound digests, and surface anything else as a normal `err` tuple so the state machine, `onError`, and `onFinish` all run. **`zsa` — `withTimeout` race catch mislabeled non-timeout errors (<ref_snippet file="packages/zsa/src/zod-safe-function.ts" lines="1044-1065" />)** Any rejection escaping `wrapper` (e.g. a `NEXT_REDIRECT` rethrown by `handleError`, or a `shapeErrorFn` throwing) flipped `timeoutStatus.isTimeout = true` and called `handleError` a second time — double-firing `onError` / `onComplete` and wrapping redirects as timeout errors. Now we rethrow redirects, only flag timeouts when the rejection is actually a `ZSAError` with `code === "TIMEOUT"`, and still run `handleError` for other unexpected failures. **`zsa-openapi` — swallowed body parse failure (<ref_snippet file="packages/zsa-openapi/src/openapi.ts" lines="428-476" />)** Malformed JSON bodies were silently dropped to `data = undefined`, which surfaced to the user as a confusing schema validation failure ("missing required field X") instead of a malformed-body error. We now read the body as text first so empty POSTs still hit schema defaults, but syntactically invalid JSON returns a 400 `BODY_PARSE_ERROR` response with a descriptive message. **`zsa-openapi` — swallowed `shapeError` failure (<ref_snippet file="packages/zsa-openapi/src/openapi.ts" lines="555-575" />)** A throwing user `shapeError` callback had its exception silently discarded while the original error was used as the response. We now still return the original error (preserving backward-compat behavior for the caller) but attach the `shapeError` throw as `cause` on the error object so the bug in the callback stays observable. **`zsa-openapi` — overly broad `parseRequest` catch (<ref_snippet file="packages/zsa-openapi/src/openapi.ts" lines="612-644" />)** The entire `parseRequest` body was wrapped in a `catch` that returned `null`, which the handler then translated into a 404. This hid genuine internal bugs (e.g. `pathToRegexp` throwing on a malformed pattern) as "no such route." Narrowed the catch to only the path-matching loop, so post-match failures now propagate through the normal error pipeline where `shapeError` / the 500 handler can see them. ### Tests - `openapi.test.tsx` → new `error handling` block: malformed JSON returns 400 via both `setupApiHandler` and `createRouteHandlers`; empty body still reaches schema defaults; throwing `shapeError` doesn't replace the original error. - `client.test.tsx` → new `useServerAction with a rejecting action` block using a new `rejecting-action` page that wires a rejecting function into the hook; verifies `execute()` resolves with an error tuple and `onError` fires instead of hanging. - `index.test.tsx` → new `next redirect` case verifying `NEXT_REDIRECT` propagates correctly through an action that declares a timeout. ## Review & Testing Checklist for Human - [ ] Confirm the new `BODY_PARSE_ERROR` → 400 response shape (`{ error, message }`) matches how you want malformed-body errors surfaced to API consumers. - [ ] Sanity-check the `zsa-react` change in a real Next.js app by triggering an RSC transport error (e.g. stop the dev server mid-request, or throw from outside a zsa handler) — `await execute()` should resolve quickly with an error tuple, not hang. - [ ] Double-check that `cause`-attachment on the shapeError fallback path doesn't collide with any shape expected by downstream tooling that serializes ZSAErrors. ### Notes - Lint warnings/errors on the `dist/*` parser-project were pre-existing on `main` and are unchanged by this PR. - The `getRetryDelay` `catch { return -1 }` is noted in the audit but intentionally left alone — its current behavior (disable retries on a throwing `delay(...)` callback) is reasonable and changing it would be a behavior change. Link to Devin session: https://localhost:3000/sessions/b2bd764bdeba4a898356912c1c22baa6 Requested by: @IdoPesok
合并状态:未合并 2 条评论