Skip to content

Implement IPC error handling and refactor command parsing in StreamFrame#1448

Open
Oluwasuyi-Timilehin wants to merge 1 commit into
dotandev:mainfrom
Oluwasuyi-Timilehin:refactor/1404-ipc-result-error-handling
Open

Implement IPC error handling and refactor command parsing in StreamFrame#1448
Oluwasuyi-Timilehin wants to merge 1 commit into
dotandev:mainfrom
Oluwasuyi-Timilehin:refactor/1404-ipc-result-error-handling

Conversation

@Oluwasuyi-Timilehin

Copy link
Copy Markdown

#closes #1404

PULL REQUEST DOCUMENTATION

================================================================================
TITLE:

fix(ipc): Bubble up IPC parsing errors instead of unwrapping

================================================================================
DESCRIPTION:

Overview

This PR updates simulator/src/ipc/mod.rs to remove unsafe unwrap() semantics during IPC command handling and serialize/deserialize operations. Instead of panicking, the IPC module now returns a custom IpcError enum when stdin read, JSON parsing, or response serialization fails.

Changes

Core Implementation

  • Added IpcError to represent IPC-specific IO, parse, and serialization failures.
  • Added parse_command_frame() to centralize JSON command parsing and return structured errors.
  • Updated handle_stdin_command() to propagate errors through Result<(), IpcError> rather than silently swallowing or panicking on invalid input.
  • Preserved SnapshotRegistry semantics while making error handling explicit.

Testing

  • Added regression test test_parse_command_frame_invalid_json_returns_error for invalid JSON command parsing.
  • Verified IPC module compilation and execution through crate-level tests.

Verification

  • Ran cargo check in simulator successfully.
  • Ran cargo test -q in simulator successfully.

Type of Change

  • New feature
  • Bug fix
  • Breaking change
  • Documentation update

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Tests added/updated
  • Documentation updated
  • No new linting issues
  • Changes verified locally

================================================================================
RELATED ISSUE

#closes

@drips-wave

drips-wave Bot commented Jun 1, 2026

Copy link
Copy Markdown

@Oluwasuyi-Timilehin Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@dotandev dotandev left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

the out file wasn't necessary, I have removed it.

@dotandev

dotandev commented Jun 1, 2026

Copy link
Copy Markdown
Owner

fix ci errors, please.

@Oluwasuyi-Timilehin

Copy link
Copy Markdown
Author

@dotandev I already fixed CI

@dotandev

dotandev commented Jun 2, 2026

Copy link
Copy Markdown
Owner

alright, thank you.

@dotandev

dotandev commented Jun 2, 2026

Copy link
Copy Markdown
Owner

wait, did you just remove the file?

there's no file for me to review again?

@Oluwasuyi-Timilehin

Copy link
Copy Markdown
Author

@dotandev No i just resolved the conflicts

@Oluwasuyi-Timilehin Oluwasuyi-Timilehin force-pushed the refactor/1404-ipc-result-error-handling branch from f6fa2ba to daabc84 Compare June 5, 2026 21:13
@Oluwasuyi-Timilehin Oluwasuyi-Timilehin force-pushed the refactor/1404-ipc-result-error-handling branch from daabc84 to 25fa305 Compare June 11, 2026 19:55
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.

Bubble up Result in IPC module

2 participants