Skip to content

OCapN: improved error messages and testing#2772

Closed
kumavis wants to merge 11 commits intomasterfrom
kumavis/ocapn-improvements
Closed

OCapN: improved error messages and testing#2772
kumavis wants to merge 11 commits intomasterfrom
kumavis/ocapn-improvements

Conversation

@kumavis
Copy link
Copy Markdown
Member

@kumavis kumavis commented Apr 24, 2025 •
edited
Loading

some small improvements:

  • strict TextDecoders
  • error chaining in codecs
  • improved test coverage

related to @endo/pass-style support of non-registered Symbols (#2777)

@kumavis kumavis force-pushed the kumavis/ocapn-improvements branch from 5de9cd8 to 3801e42 Compare April 24, 2025 19:04
@kumavis kumavis changed the title Kumavis/ocapn improvements OCapN: improved error messages and testing Apr 24, 2025
@kumavis kumavis requested a review from Copilot April 24, 2025 19:06
Copilot AI reviewed Apr 24, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR improves error messages and testing support across the codebase by adding error wrapping (with nested causes) and stricter decorator-based validations, as well as expanding test coverage for various codecs and passable values. Key changes include:

  • Wrapping codec read/write methods with detailed error messages.
  • Updating test expectations to use a new throws utility and a less-strict TextDecoder for fuzz tests.
  • Renaming and prefixing codec identifiers for clarity and consistency.

Reviewed Changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/ocapn/test/syrup/* Updated tests to use a non-fatal TextDecoder for fuzz descriptor and standardized error expectations via throws utility
packages/ocapn/src/syrup/* Refactored codec implementations with error wrapping and added codec name parameters
packages/ocapn/src/codecs/* Improved error chaining, renamed codecs with consistent prefixes, and updated union codec implementations
packages/ocapn/src/syrup/buffer-reader.js Fixed a formatting bug in an error message
packages/ocapn/test/_table.js & _syrup_util.js Adjusted test table entries and moved utility functions for consistency
Comments suppressed due to low confidence (2)

packages/ocapn/src/syrup/buffer-reader.js:110

  • The error message now includes a closing parenthesis, which fixes the previous formatting issue. Verify that all similar error messages follow this corrected punctuation for consistency.
        `End of data reached (data length = ${state.length}, asked index ${index})`

packages/ocapn/test/syrup/_table.js:13

  • There is a duplicate table entry with syrup '1-' and value -1n. Confirm whether this duplication is intentional or if the redundant entry should be removed.
    { syrup: '1-', value: -1n },

@kumavis kumavis marked this pull request as ready for review April 24, 2025 19:08
kriskowal
kriskowal approved these changes Apr 24, 2025
View reviewed changes
erights
erights requested changes Apr 24, 2025
View reviewed changes
@kumavis kumavis mentioned this pull request Apr 24, 2025
Closed
@kumavis
Copy link
Copy Markdown
Member Author

kumavis commented Apr 25, 2025 •
edited
Loading

@erights in order to avoid blocking this PR on @endo/pass-style Selector support, i have added a file to provide the minimal functionality needed from @endo/pass-style for this early implementation of OCapN (currently focused on message parsing). After Selector support is added to @endo/pass-style this file will be easily replaced with the methods of the same name provided by @endo/pass-style

@erights erights mentioned this pull request Apr 26, 2025
Closed
2 tasks
@kumavis kumavis mentioned this pull request Apr 28, 2025
Closed
@kumavis kumavis requested a review from erights April 29, 2025 17:38
@kumavis
Copy link
Copy Markdown
Member Author

kumavis commented Apr 29, 2025

@erights since #2777 has been moved to draft mode, I'd like to move forward on this PR without blocking on it. This package (ocapn) is unpublished and I can make the commitment to updating the overlap with pass-style before release

@kumavis kumavis force-pushed the kumavis/ocapn-improvements branch from fe3ebc4 to 41f429e Compare April 29, 2025 17:43
@kumavis
Copy link
Copy Markdown
Member Author

kumavis commented Apr 29, 2025

@kriskowal has suggested i use registered symbols in the interim

@kumavis kumavis marked this pull request as draft April 29, 2025 19:40
@kumavis
Copy link
Copy Markdown
Member Author

kumavis commented Apr 29, 2025 •
edited
Loading

I decided to just include these changes in #2781 and keep working for awhile

@kumavis kumavis closed this Apr 29, 2025
@erights
Copy link
Copy Markdown
Contributor

erights commented Apr 29, 2025

I decided to just include these changes in #2772 and keep working for awhile

I don't understand. #2772 is this PR which you just closed. Did you mean another PR?

@kumavis
Copy link
Copy Markdown
Member Author

kumavis commented Apr 29, 2025

sorry - #2781

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kriskowal kriskowal kriskowal approved these changes

Copilot code review Copilot Copilot left review comments

@erights erights Awaiting requested review from erights

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kumavis @erights @kriskowal