feat: throw UnwrapException with value context from unwrap()/unwrapErr()#94
Conversation
unwrap() on Err and unwrapErr() on Ok should throw a dedicated UnwrapException (extending LogicException for BC) whose message includes a description of the contained value, mirroring Rust's panic message which includes the Debug representation. Currently RED: the implementation throws plain LogicException without value context. Claude-Session: https://claude.ai/code/session_017XTM7pxbWPVNLV639i5WgK
Introduce a dedicated UnwrapException so callers can catch unwrap misuse specifically, and include a description of the contained value in the message (class + message for Throwables, class + string for Stringables, class name for other objects, var_export for scalars), mirroring Rust's panic output for Result::unwrap(). UnwrapException extends LogicException, so existing catch sites and the previous message prefix keep working. Claude-Session: https://claude.ai/code/session_017XTM7pxbWPVNLV639i5WgK
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #94 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 40 54 +14
===========================================
Files 2 3 +1
Lines 80 107 +27
===========================================
+ Hits 80 107 +27
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a custom UnwrapException (extending \LogicException) to provide detailed context when calling unwrap() on an Err value or unwrapErr() on an Ok value, mimicking Rust's panic messages. The changes update the implementation, documentation, and tests accordingly. The review feedback suggests improving robustness in UnwrapException::describe() by handling potential exceptions thrown during Stringable string casting to avoid masking the original exception, and adding a corresponding test case to verify this fallback behavior.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| private static function describe(mixed $value): string | ||
| { | ||
| return match (true) { | ||
| $value instanceof \Throwable => \sprintf('%s: %s', $value::class, $value->getMessage()), | ||
| $value instanceof \Stringable => \sprintf('%s: %s', $value::class, (string) $value), | ||
| \is_object($value) => $value::class, | ||
| \is_scalar($value), null === $value => var_export($value, true), | ||
| default => get_debug_type($value), | ||
| }; | ||
| } |
There was a problem hiding this comment.
Stringable インターフェースを実装したオブジェクトを文字列キャストする際、__toString() メソッド内で例外（Throwable）がスローされる可能性があります。
もし __toString() が例外をスローした場合、本来スローされるべき UnwrapException がその例外によって上書き（マスク）されてしまい、unwrap 失敗のデバッグが困難になります。
これを防ぐため、Stringable のキャスト処理を try-catch ブロックで囲み、例外が発生した場合はクラス名のみを返すようにフォールバックする設計に改善することをお勧めします。
private static function describe(mixed $value): string
{
return match (true) {
$value instanceof \\Throwable => \\sprintf('%s: %s', $value::class, $value->getMessage()),
$value instanceof \\Stringable => self::describeStringable($value),
\\is_object($value) => $value::class,
\\is_scalar($value), null === $value => \\var_export($value, true),
default => \\get_debug_type($value),
};
}
private static function describeStringable(\\Stringable $value): string
{
try {
return \\sprintf('%s: %s', $value::class, (string) $value);
} catch (\\Throwable) {
return $value::class;
}
}| #[Test] | ||
| public function unwrapErr_withStringableValue_includesClassAndString(): void | ||
| { | ||
| $value = new class () implements \Stringable { | ||
| public function __toString(): string | ||
| { | ||
| return 'stringable value'; | ||
| } | ||
| }; | ||
| $ok = new Ok($value); | ||
| $this->expectException(UnwrapException::class); | ||
| $this->expectExceptionMessage('stringable value'); | ||
| $ok->unwrapErr(); | ||
| } |
There was a problem hiding this comment.
__toString() が例外をスローした場合でも、UnwrapException が正しくスローされ、クラス名にフォールバックされることを検証するテストケースを追加することをお勧めします。
#[Test]
public function unwrapErr_withStringableValue_includesClassAndString(): void
{
$value = new class () implements \\Stringable {
public function __toString(): string
{
return 'stringable value';
}
};
$ok = new Ok($value);
$this->expectException(UnwrapException::class);
$this->expectExceptionMessage('stringable value');
$ok->unwrapErr();
}
#[Test]
public function unwrapErr_withStringableValueThatThrows_fallsBackToClassName(): void
{
$value = new class () implements \\Stringable {
public function __toString(): string
{
throw new \\RuntimeException('toString failed');
}
};
$ok = new Ok($value);
$this->expectException(UnwrapException::class);
$this->expectExceptionMessage('called Result::unwrapErr() on an Ok value:');
$ok->unwrapErr();
}There was a problem hiding this comment.
Pull request overview
This PR improves the ergonomics and debuggability of Result::unwrap() / Result::unwrapErr() failures by introducing a dedicated UnwrapException (extending LogicException) and including a summarized representation of the held value in the exception message, plus corresponding documentation and tests.
Changes:
- Add
Valbeat\Result\UnwrapExceptionwith helper constructors and value summarization for exception messages. - Switch
Err->unwrap()andOk->unwrapErr()to throwUnwrapExceptioninstead of a rawLogicException. - Update
Resultphpdoc (@throws) and README, and add PHPUnit coverage for message formatting/catchability.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/OkTest.php | Adds tests asserting unwrapErr() throws UnwrapException and includes value context in the message. |
| tests/ErrTest.php | Adds tests asserting unwrap() throws UnwrapException, message formatting for different error types, and catchability as LogicException. |
| src/UnwrapException.php | New exception type with message construction and value summarization logic. |
| src/Result.php | Adds @throws UnwrapException annotations to unwrap() / unwrapErr(). |
| src/Ok.php | Updates unwrapErr() to throw UnwrapException with value context. |
| src/Err.php | Updates unwrap() to throw UnwrapException with error context. |
| README.md | Updates documentation to mention UnwrapException instead of LogicException. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private static function describe(mixed $value): string | ||
| { | ||
| return match (true) { | ||
| $value instanceof \Throwable => \sprintf('%s: %s', $value::class, $value->getMessage()), | ||
| $value instanceof \Stringable => \sprintf('%s: %s', $value::class, (string) $value), | ||
| \is_object($value) => $value::class, | ||
| \is_scalar($value), null === $value => var_export($value, true), | ||
| default => get_debug_type($value), | ||
| }; | ||
| } |
| try { | ||
| $err->unwrap(); | ||
| } catch (\LogicException $e) { | ||
| $this->assertInstanceOf(UnwrapException::class, $e); | ||
| } | ||
| } |
- a Stringable whose __toString() throws must not replace UnwrapException with the escaping exception - enum error values must keep their case name in the message - long string values must be truncated to bound message size - multiline string values must not break single-line log formats Currently RED. Claude-Session: https://claude.ai/code/session_017XTM7pxbWPVNLV639i5WgK
- a Stringable whose __toString() throws no longer replaces UnwrapException with the escaping exception (falls back to the class name) - enum error values keep their case name (Status::NotFound) instead of collapsing to the bare class name - summaries are normalized to a single line and truncated at 120 chars, bounding message size and keeping log lines intact - anonymous class names drop the file-path suffix so the value part is not pushed past the truncation limit Claude-Session: https://claude.ai/code/session_017XTM7pxbWPVNLV639i5WgK
Stack this PR on #94 per review, resolving overlapping test and README hunks. expect()/expectErr() switch to UnwrapException in the following commits. Claude-Session: https://claude.ai/code/session_017XTM7pxbWPVNLV639i5WgK
概要
コードレビューで指摘した例外設計の改善です。現状
unwrap()/unwrapErr()は素のLogicExceptionを投げるため、(1) 利用側が unwrap 失敗だけを選択的に catch できず、(2) メッセージに保持値の情報がなくデバッグ時に不便でした（Rust の panic はエラー値の Debug 表現を含みます）。変更内容
UnwrapException（LogicExceptionを継承）を新設Err->unwrap()/Ok->unwrapErr()はこの例外を投げ、メッセージに値の要約を含める:Throwable: クラス名 + メッセージ（例:RuntimeException: boom）Stringable: クラス名 + 文字列表現var_export表現get_debug_typeunwrap()/unwrapErr()に@throws UnwrapExceptionを追記BC への影響
なし。
LogicExceptionを継承し、従来のメッセージはプレフィックスとして保持しているため、既存のcatch (\LogicException)やexpectExceptionMessage('called Result::unwrap() on an Err value')はそのまま通ります（既存テストは無変更でパス）。TDD
https://claude.ai/code/session_017XTM7pxbWPVNLV639i5WgK