Skip to content

Add associated Error type to Writable traits#597

Merged
reaperhulk merged 1 commit intomainfrom
claude/add-writable-error-type-dAyLZ
Feb 25, 2026
Merged

Add associated Error type to Writable traits#597
reaperhulk merged 1 commit intomainfrom
claude/add-writable-error-type-dAyLZ

Conversation

@alex
Copy link
Owner

@alex alex commented Feb 16, 2026

Add type Error: From<WriteError> to Asn1Writable, SimpleAsn1Writable, and Asn1DefinedByWritable. Update all implementations, the derive macros, writer infrastructure, and Choice macro accordingly.

Derive bounds use loose From-based error conversion for regular fields and Asn1Writable<Error = WriteError> for tagged (implicit/explicit) fields to preserve perfect derive support.

Includes a test demonstrating a custom error type.

https://claude.ai/code/session_01BBujUNHUboN5Rv6MkVbbxU

@alex alex force-pushed the claude/add-writable-error-type-dAyLZ branch from d032588 to 8d52b93 Compare February 16, 2026 19:22
Add `type Error: From<WriteError>` to `Asn1Writable`,
`SimpleAsn1Writable`, and `Asn1DefinedByWritable`. Update all
implementations, the derive macros, writer infrastructure, and
Choice macro accordingly.

Derive bounds use loose `From`-based error conversion for regular
fields and `Asn1Writable<Error = WriteError>` for tagged
(implicit/explicit) fields to preserve perfect derive support.

Includes a test demonstrating a custom error type.

https://claude.ai/code/session_01BBujUNHUboN5Rv6MkVbbxU
@alex alex force-pushed the claude/add-writable-error-type-dAyLZ branch from 8d52b93 to 3027716 Compare February 16, 2026 19:25
@alex
Copy link
Owner Author

alex commented Feb 24, 2026

@facutuesca I think this gets us started down the path we need.

@facutuesca
Copy link
Contributor

@facutuesca I think this gets us started down the path we need.

awesome!

@facutuesca
Copy link
Contributor

@alex how should we handle error types for SequenceWriter ? Here, SequenceWriter::Error = WriteError, but when we pass a callback to write each field in Sequence::new(), each of those fields does not necessarily return WriteError.

@alex
Copy link
Owner Author

alex commented Feb 24, 2026

Follow up PR to make SequenceWriter be generic over Error, I guess?

@facutuesca
Copy link
Contributor

Follow up PR to make SequenceWriter be generic over Error, I guess?

I created a draft PR for that here: #599

I also created a branch in cryptography with the changes needed after this PR and #599 are merged: https://github.com/trail-of-forks/cryptography/tree/ft/asn1-custom-error

@alex
Copy link
Owner Author

alex commented Feb 24, 2026 via email

@facutuesca
Copy link
Contributor

And that's sufficient?

I think so? All the tests pass. The only thing missing from that branch is actually bumping the rust-asn1 version in Cargo.toml once a new version is released.

@alex
Copy link
Owner Author

alex commented Feb 24, 2026 via email

Copy link
Collaborator

@reaperhulk reaperhulk left a comment

Choose a reason for hiding this comment

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

to the extent that any macro is reviewable I have reviewed this 😛

@reaperhulk reaperhulk merged commit ced0432 into main Feb 25, 2026
13 checks passed
@reaperhulk reaperhulk deleted the claude/add-writable-error-type-dAyLZ branch February 25, 2026 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants