Skip to content

feat: Marshal and unmarshal @ocap/errors - #154

Merged
sirtimid merged 24 commits into
mainfrom
sirtimid/un-marshal-custom-errors
Oct 17, 2024
Merged

feat: Marshal and unmarshal @ocap/errors#154
sirtimid merged 24 commits into
mainfrom
sirtimid/un-marshal-custom-errors

Conversation

@sirtimid

@sirtimidsirtimid commented Oct 14, 2024

Copy link
Copy Markdown
Member

closes#150

Following #149, this PR modifies the marshaling functions in @ocap/streams such that errors from @ocap/errors are unmarshaled into their respective classes, based on the error code. It also extends the stringify utility to support ocap errors.

@sirtimid
sirtimidforce-pushed the sirtimid/un-marshal-custom-errors branch 2 times, most recently from 07f97ca to 3c7db2dCompareOctober 14, 2024 17:49
@sirtimidsirtimid mentioned this pull request Oct 14, 2024
@sirtimid
sirtimidforce-pushed the sirtimid/un-marshal-custom-errors branch from 7f0cadf to b622374CompareOctober 15, 2024 19:22
Base automatically changed from sirtimid/custom-error-classes to mainOctober 15, 2024 22:53
@sirtimid
sirtimidforce-pushed the sirtimid/un-marshal-custom-errors branch from b622374 to 77d6ca6CompareOctober 16, 2024 14:29
@sirtimid
sirtimid marked this pull request as ready for review October 16, 2024 14:29
@sirtimid
sirtimid requested a review from a team as a code ownerOctober 16, 2024 14:29

@rekmarksrekmarks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice! I have various questions and suggestions. For all of the errors, I left a number of suggestions on VatNotFoundError that are generally applicable.

Two big changes are harden:ing the error prototypes, error instances, and marshaled errors. You may want to do that after any changes I propose about e.g. reorganizing the types / utils.

Comment threadpackages/kernel/src/stream-envelope.ts Outdated
Comment threadpackages/errors/src/errors/index.ts Outdated
Comment threadpackages/errors/src/types.ts
Comment threadpackages/errors/src/marshal/isMarshaledOcapError.ts Outdated
Comment threadpackages/errors/src/marshal/unmarshalError.test.ts Outdated
Comment threadpackages/errors/src/errors/VatNotFoundError.ts
Comment threadpackages/errors/src/errors/VatNotFoundError.ts Outdated
Comment threadpackages/errors/src/errors/VatNotFoundError.ts
Comment threadpackages/errors/src/errors/VatNotFoundError.ts
Comment threadpackages/errors/src/marshal/marshalError.ts Outdated
@sirtimid
sirtimidforce-pushed the sirtimid/un-marshal-custom-errors branch from e8d9eeb to b587495CompareOctober 16, 2024 20:05
@sirtimid
sirtimidforce-pushed the sirtimid/un-marshal-custom-errors branch from ff208cf to 93c4084CompareOctober 17, 2024 14:23
@sirtimid
sirtimidforce-pushed the sirtimid/un-marshal-custom-errors branch from 93c4084 to 4a455f3CompareOctober 17, 2024 15:40

@rekmarksrekmarks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just a couple more things.

Comment threadpackages/errors/src/constants.ts Outdated
Comment on lines +31 to +53
/**
* Struct to validate marshaled errors.
*/
export const MarshaledErrorStruct = object({
[ErrorSentinel]: literal(true),
message: string(),
code: optional(ErrorCodeStruct),
data: optional(JsonStruct),
stack: optional(string()),
cause: optional(union([string(), lazy(() => MarshaledErrorStruct)])),
}) as Struct<MarshaledError>;

/**
* Base schema for validating Ocap error classes during error marshaling.
*/
export const baseErrorStructSchema = {
[ErrorSentinel]: literal(true),
message: string(),
code: ErrorCodeStruct,
data: JsonStruct,
stack: optional(string()),
cause: optional(union([string(), lazy(() => MarshaledErrorStruct)])),
};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See comment in isMarshaledOcapError.ts.

Suggested change
/**
*Structtovalidatemarshalederrors.
*/
exportconstMarshaledErrorStruct=object({
[ErrorSentinel]: literal(true),
message: string(),
code: optional(ErrorCodeStruct),
data: optional(JsonStruct),
stack: optional(string()),
cause: optional(union([string(),lazy(()=>MarshaledErrorStruct)])),
})asStruct<MarshaledError>;
/**
*BaseschemaforvalidatingOcaperrorclassesduringerrormarshaling.
*/
exportconstbaseErrorStructSchema={
[ErrorSentinel]: literal(true),
message: string(),
code: ErrorCodeStruct,
data: JsonStruct,
stack: optional(string()),
cause: optional(union([string(),lazy(()=>MarshaledErrorStruct)])),
};
constmarshaledErrorSchema={
[ErrorSentinel]: literal(true),
message: string(),
data: optional(JsonStruct),
stack: optional(string()),
};
/**
*Structtovalidatemarshalederrors.
*/
exportconstMarshaledErrorStruct=object({
...marshaledErrorSchema,
cause: optional(union([string(),lazy(()=>MarshaledErrorStruct)])),
})asStruct<MarshaledError>;
/**
*Structtovalidatemarshaledocaperrors.
*/
exportconstMarshaledOcapErrorStruct=object({
...marshaledErrorSchema,
code: ErrorCodeStruct,
data: JsonStruct,
cause: optional(union([string(),lazy(()=>MarshaledOcapErrorStruct)])),
})asStruct<MarshaledOcapError>;
/**
*BaseschemaforvalidatingOcaperrorclassesduringerrormarshaling.
*/
exportconstbaseErrorStructSchema={
[ErrorSentinel]: literal(true),
message: string(),
code: ErrorCodeStruct,
data: JsonStruct,
stack: optional(string()),
cause: optional(union([string(),lazy(()=>MarshaledErrorStruct)])),
};

Comment threadpackages/errors/src/marshal/isMarshaledOcapError.ts Outdated
Comment threadpackages/errors/src/BaseError.ts

@rekmarksrekmarks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@sirtimid
sirtimid merged commit 474d285 into mainOct 17, 2024
@sirtimid
sirtimid deleted the sirtimid/un-marshal-custom-errors branch October 17, 2024 19:32
Sign up for freeto 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.

Marshal and unmarshal @ocap/errors

2 participants

@sirtimid@rekmarks