Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces internal error handling improvements by refactoring the error normalization system. The main changes involve:
- Renaming the
onEnsureplugin hook toonUnknownfor better semantic clarity - Introducing a new internal error code
errata.unknown_errorto replace ad-hoc error codes likebe.unknown_error,be.deserialization_failed, andbe.network_error - Adding a new
onSerializeplugin hook for payload transformation during serialization - Improving type narrowing behavior to maintain precise types when working with locally created errors
- Adding an
onUnknownconfiguration option to both server and client instances for custom error mapping
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/errata/test/plugins.test.ts | Updates plugin tests to use renamed onUnknown hook instead of onEnsure, adds test for new onSerialize hook, and updates error log expectations |
| packages/errata/test/pattern-matching.test.ts | Adjusts type narrowing tests to reflect that locally created errors maintain narrow types, and adds explicit ensure() calls where type widening is needed |
| packages/errata/test/errata.test.ts | Adds comprehensive tests for the new onUnknown configuration option and updates expectations for internal error codes |
| packages/errata/test/client.test.ts | Updates client-side tests to use errata.unknown_error instead of backend-specific codes, adds tests for client onUnknown hook |
| packages/errata/test/client-plugins.test.ts | Updates error code expectations from be.deserialization_failed to errata.unknown_error |
| packages/errata/src/types.ts | Introduces InternalCode and InternalDetails types, refactors pattern matching types to work with string unions, adds onSerialize hook definition |
| packages/errata/src/index.ts | Exports new types including InternalCode, InternalDetails, and various helper types for working with both user and internal codes |
| packages/errata/src/errata.ts | Implements internal error handling with errata.unknown_error, adds onUnknown option, implements onSerialize hook execution, updates type signatures |
| packages/errata/src/client.ts | Refactors client to use errata.unknown_error, adds client-side onUnknown option, implements ensure() method, simplifies safe() implementation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it('handles errors thrown in onEnsure gracefully', () => { | ||
| const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) | ||
| it('handles errors thrown in onUnknown gracefully', () => { | ||
| const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => { }) |
There was a problem hiding this comment.
The empty arrow function implementation has been changed from () => {} to () => { }. This formatting change adds an unnecessary space inside empty braces, which is inconsistent with common JavaScript/TypeScript formatting conventions. Most style guides recommend no space in empty braces for consistency.
| const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => { }) | |
| const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) |
|
|
||
| it('swallows errors thrown in onCreate without crashing', () => { | ||
| const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) | ||
| const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => { }) |
There was a problem hiding this comment.
The empty arrow function implementation has been changed from () => {} to () => { }. This formatting change adds an unnecessary space inside empty braces, which is inconsistent with common JavaScript/TypeScript formatting conventions. Most style guides recommend no space in empty braces for consistency.
| const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => { }) | |
| const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) |
|
|
||
| it('warns on duplicate plugin names', () => { | ||
| const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) | ||
| const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => { }) |
There was a problem hiding this comment.
The empty arrow function implementation has been changed from () => {} to () => { }. This formatting change adds an unnecessary space inside empty braces, which is inconsistent with common JavaScript/TypeScript formatting conventions. Most style guides recommend no space in empty braces for consistency.
| const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => { }) | |
| const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) |
|
|
||
| it('warns on code collision between plugins and base codes', () => { | ||
| const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) | ||
| const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => { }) |
There was a problem hiding this comment.
The empty arrow function implementation has been changed from () => {} to () => { }. This formatting change adds an unnecessary space inside empty braces, which is inconsistent with common JavaScript/TypeScript formatting conventions. Most style guides recommend no space in empty braces for consistency.
No description provided.