-
Notifications
You must be signed in to change notification settings - Fork 76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(sdks/actor/runtime): fix internal errors not getting logged & make UserError public #1958
Conversation
…e UserError public
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
Improved error handling and logging in the actor runtime by centralizing error management and adjusting error visibility.
- Removed
public: true
fromStateTooLarge
error insdks/actor/runtime/src/errors.ts
to prevent exposing memory limitations - Added explicit
public: true
flag toUserError
constructor for consistent user-facing error handling - Centralized error logging in
sdks/actor/runtime/src/event.ts
with different log levels for public (info) and internal (warn) errors - Removed redundant
onError
handlers from WebSocket handling insdks/actor/runtime/src/actor.ts
andsdks/actor/runtime/src/inspect.ts
- Added connection and RPC context to error logs for better debugging in
sdks/actor/runtime/src/event.ts
4 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
@@ -174,7 +174,7 @@ export interface UserErrorOptions extends ErrorOptions { | |||
metadata: unknown; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: metadata is marked as required but opts is optional - this could cause runtime errors if opts is undefined
logger().warn("internal error", { | ||
error: String(error), | ||
connectionId: conn.id, | ||
rpcId, | ||
rpcName, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: consider adding stack trace to internal error logs for better debugging
Merge activity
|
…e UserError public (#1958) <!-- Please make sure there is an issue that this PR is correlated to. --> ## Changes <!-- If there are frontend changes, please include screenshots. -->
Changes