Improve error handling: stop silently swallowing/masking errors across zsa, zsa-openapi, and zsa-react
## Summary
Reviewed error handling across the monorepo and fixed several places where errors were silently swallowed or could mask/replace the "real" error that callers care about. All existing tests still pass (130 → 133 with the new ones added here).
### Fixes
**`packages/zsa/src/zod-safe-function.ts`**
- `getRetryDelay` previously had a bare `} catch { return -1 }` that swallowed any error from a user-supplied `retry.delay(...)` callback. Now the user delay callback is wrapped in its own try/catch and the error is surfaced via `console.error` before falling back to "no retry". The outer catch-all also logs instead of silently swallowing.
- `handleError` awaited every user-supplied `onError` / `onComplete` callback without a try/catch. A throw inside such a callback would bubble out of `handleError` and **replace the original action error** that propagates up to the caller. Each callback is now wrapped so a callback bug is logged but never masks or replaces the action's real error.
- `handleSuccess` had the same pattern for `onSuccess` / `onComplete`. A throw inside a success-side side-effect callback (logging, analytics, etc.) would be caught by the outer wrapper and **turn a successful action into a failure**. Success callbacks are now isolated: a throw is logged via `console.error` but the successful result is returned unchanged.
**`packages/zsa-openapi/src/openapi.ts`**
- The body-parse `try/catch` around `request.clone().json()` / `.formData()` previously collapsed any parse failure into `data = undefined`, letting the action run and produce a misleading `INPUT_PARSE_ERROR` for what was actually malformed JSON. Now a *present but malformed* body (detected via `content-length` / `transfer-encoding`) surfaces a new `BODY_PARSE_ERROR` marker and the handler returns a real `400 { error: "Invalid request body" }`. Missing/empty bodies keep the legacy "no input" behavior so schema defaults still work.
- `try { error = shapeError(error) } catch (error: any) { error = $error }` silently discarded any error from a user-supplied `shapeError` callback. The callback error is now logged via `console.error` (so the bug is observable) before falling back to the original error.
- `parseRequest` wrapped its whole body in `try { ... } catch (error: unknown) { return null }`. A `null` return means "no route matched" and the handler surfaces a 404 — so any internal request-parsing error showed up as a misleading 404 with no log. The handler now distinguishes "no match" (still `null` → 404) from "internal parse failure" (new `{ internalError }` → 500), and logs the underlying error.
**`packages/zsa-react/src/index.ts`**
- `await serverAction(...).then(...)` inside `internalExecute` had no catch. If the server action threw instead of returning a `[data, err]` tuple (network failure, unexpected runtime error), the rejection escaped `startTransition`, the `executeRef.current` resolver never fired, and `execute()` hung forever. The thrown error is now normalized into a `ZSAError`-shaped object so callers always get a deterministic tuple and `isExecuting` is cleared.
- The retry `setTimeout` did `internalExecute(...).then(resolve)` with no `.catch`. Same hang-forever risk on the retry path. Now has a `.catch` that resolves with an error tuple instead of leaving the outer `Promise` unresolved.
### Tests added
- `tests/jest/__tests__/index.test.tsx`: "does not mask the original error when an onError callback throws" and "does not turn a successful action into a failure when an onSuccess callback throws".
- `tests/jest/__tests__/openapi.test.tsx`: "should return 400 when the JSON body is malformed [POST]".
Full jest suite: **133 passed, 0 failed**.
## Review & Testing Checklist for Human
- [ ] Confirm that surfacing `BODY_PARSE_ERROR` as a 400 is the desired behavior. This is technically a behavior change for callers that relied on sending malformed JSON and getting `INPUT_PARSE_ERROR` back. Empty-body requests (legacy tests for `multiplyWithDefaultValues` / `multiplyWithDefaultObject`) still return 200.
- [ ] Review the `console.error` calls added for user-callback failures (`onError`, `onSuccess`, `onComplete`, `shapeError`, `retry.delay`). If you'd prefer these be silent or routed through a configurable logger instead, let me know and I'll swap it out.
- [ ] Spot-check the zsa-react change: server actions that previously threw (instead of returning `[data, err]`) now surface a typed error via the client hook rather than hanging. If any existing UI relies on the hang (unlikely), that would change.
- [ ] Test plan: `npm install && npm run build && npm run test` (all 133 jest tests pass locally; playwright runs in CI).
### Notes
- No dependency changes. No existing behavior intentionally broken: empty POST bodies, NEXT_REDIRECT, NEXT_NOT_FOUND paths all still pass their existing tests.
- I intentionally did not touch `handleStart` — callbacks there run before the action truly begins, so routing a throw through `handleError` is arguably correct behavior rather than silent swallowing.
Link to Devin session: https://localhost:3000/sessions/4f09454dce874764ab935ea6b5e7b169
Requested by: @IdoPesok
合并状态:未合并 3 条评论