fix(pic): fail immediately on non-retryable PocketIC server errors#242
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates PicJS retry behavior to fail fast on non-retryable PocketIC server failures by introducing explicit retryable vs non-retryable error types, and updating polling logic/tests accordingly.
Changes:
- Add
RetryableErrorandServerErrordistinctions; update polling to only retryRetryableError. - Update
Http2Clientto throwRetryableErrorfor busy/processing states andServerErrorfor definitive server errors / parse failures. - Add unit/integration tests and export
ServerErroras part of the public API.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Enables noImplicitOverride to enforce explicit override usage. |
| packages/pic/src/error.ts | Adds RetryableError, makes BinTimeoutError retryable, introduces ServerError, and annotates error name overrides. |
| packages/pic/src/util/poll.ts | Changes polling to retry only RetryableError and fail immediately otherwise. |
| packages/pic/src/http2-client.ts | Throws RetryableError for retryable states and ServerError for definitive server failures / parse failures. |
| packages/pic/src/pocket-ic-server.ts | Adjusts port-file polling to throw retryable timeout errors; adds override on NullStream._write. |
| packages/pic/src/index.ts | Exports ServerError from the package entrypoint. |
| packages/pic/tests/src/util/poll.spec.ts | Adds tests validating retry/fail-fast behavior for poll. |
| packages/pic/tests/src/util/http2-client.spec.ts | Adds tests validating fail-fast ServerError behavior and retry on busy responses. |
| packages/pic/tests/src/util/error.spec.ts | Adds tests for RetryableError and ServerError semantics/properties. |
| packages/pic/tests/src/time.spec.ts | Adds regression test ensuring setTime into the past fails quickly (no long polling). |
| CHANGELOG.md | Documents the fail-fast behavior change under Unreleased fixes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
nikosxenakis
deleted the
nikosxenakis/SDK-2080-fail-fast-on-server-errors
branch
Motivation
PicJS retries all failed requests until they time out, making it hard to get feedback when a call is malformed or hits a definitive server error.
Summary
This PR introduces a RetryableError / ServerError distinction. The polling loop now only retries RetryableError. Any other error, including ServerError, which the server returns when a request is definitively invalid, propagates immediately.
ServerError is also exported as part of the public API so callers can catch it by type and inspect .serverMessage directly, rather than matching against the error message string.
SDK-2080