-
Notifications
You must be signed in to change notification settings - Fork 0
Make Result to catch any type with F extends Object #23
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request primarily enhances type safety and flexibility across the codebase by introducing a generic error type parameter Changes
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/src/fold.dart (1)
4-4
: Consider a more descriptive generic type name.
UsingF extends Object
is perfectly valid for allowing broad failure types, but naming the genericF
as something likeFailureType
orE
(commonly used for exceptions) could improve readability for contributors new to this pattern.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
bench/record_result.dart
(1 hunks)example/inline_result_example.dart
(1 hunks)lib/src/failure.dart
(1 hunks)lib/src/fold.dart
(1 hunks)lib/src/future.dart
(1 hunks)lib/src/getter.dart
(2 hunks)lib/src/on_action.dart
(2 hunks)lib/src/or.dart
(0 hunks)lib/src/recover.dart
(3 hunks)lib/src/result.dart
(2 hunks)lib/src/run_catching.dart
(4 hunks)lib/src/transformation.dart
(1 hunks)test/inline_result_test.dart
(21 hunks)
💤 Files with no reviewable changes (1)
- lib/src/or.dart
🔇 Additional comments (80)
bench/record_result.dart (1)
1-1
: Added lint ignore for null checks is necessaryThis lint ignore is necessary as the file performs multiple null checks on nullable type parameters, particularly on
_value.$1
and_value.$2
throughout the extension methods. This line suppresses the warnings about performing null checks on nullable type parameters.example/inline_result_example.dart (1)
5-5
: Return type updated to use the new generic error parameterThe signature change from
Result<int>
toResult<int, Object>
reflects the new type parameter for error handling. This correctly implements the PR's goal of makingResult
work with any type extendingObject
. The change is backward compatible as all usages in themain()
method continue to work without modification.lib/src/failure.dart (3)
5-5
: Added generic type parameter for flexible error type handlingThe addition of the generic type parameter
F extends Object
to the_Failure
class allows it to handle any error type, not justException
. This is a key change that supports the PR's goal of making theResult
type more flexible.
7-7
: Updated exception field type to use the generic parameterThe field type change from
Exception
toF
ensures that the class can store any error type specified through the type parameter. This complements the class declaration change and maintains type safety throughout the error handling flow.
26-29
:❓ Verification inconclusive
Verify that equality comparison works for all Object types
The equality operator remains unchanged but will now compare different types of exceptions. This should work fine for most cases as it relies on the
==
operator of the exception objects themselves, but be aware that some custom exception types might have inconsistent equality implementations.
🏁 Script executed:
#!/bin/bash # Let's check if the codebase has tests verifying the equality operator with different exception types # Search for test code that might test equality with different exception types rg -A 3 "operator ==" -g "test/**/*.dart" # Check for tests specifically related to Result equality with custom exception types rg -A 10 "test.*Result.*equals" -g "test/**/*.dart" rg -A 10 "expect.*Result.*equal" -g "test/**/*.dart"Length of output: 2493
Focus on Custom Exception Equality in _Failure
The equality operator in
lib/src/failure.dart
(lines 26–29) compares both theexception
and thestacktrace
, relying on each exception’s own==
implementation. Verification via tests intest/inline_result_test.dart
(which use aCustomException
) shows typical usage, but note that these tests do not explicitly validate the equality operator for_Failure
across a range of exception types. If any custom exception does not implement equality consistently, the comparison may yield unexpected results. It might be worthwhile to add direct tests for failure cases involving various custom exception types.lib/src/future.dart (2)
32-36
:✅ Verification successful
Updated FutureOrResult.asResult to use generic error type
The implementation of
asResult
inFutureOrResult
now uses the generic type parameterF
to catch specific exception types. This change is consistent with the changes toFutureResult
and ensures that both extensions handle errors in the same way.Notice that the try-catch block now catches only exceptions of type
F
. This means if a different exception type is thrown, it won't be caught by this block but will propagate up the call stack. Ensure this is the intended behavior.
🌐 Web query:
What happens in Dart when an exception doesn't match the catch clause type?
💡 Result:
In Dart, when an exception isn't matched by any
catch
clause type, it propagates up the call stack until it finds a matching handler. If no handler is found, the isolate where the exception occurred is suspended, and the program typically terminates[1]. Here's a breakdown of the behavior:
Exception Handling Flow
Unmatched Exceptions
If an exception type isn't specified in anon
clause or caught by a generalcatch
, it bypasses the currenttry/catch
block:try { throw FormatException('Invalid data'); } on ArgumentError catch (e) { // This won't catch FormatException } // Exception propagates beyond this blockIsolate Suspension
Dart runs code in isolated memory heaps called isolates. Uncaught exceptions suspend the isolate that threw them, halting further execution in that isolate[1]. This usually terminates the entire program unless other isolates are active.No Checked Exceptions
Unlike Java, Dart has no "checked exceptions" concept. Methods don't declare thrown exceptions, and you aren't forced to handle them:void riskyMethod() { throw 'Unplanned error'; // No compile-time checks }
Key Differences from Other Languages
Aspect Dart Behavior Exception Matching Uses first compatible on
/catch
clause; unmatched exceptions propagate.Error Types Any non-null object can be thrown (e.g., throw 'Error!'
)[1][8].Program Termination Unhandled exceptions terminate the isolate (and often the entire app)[1].
Best Practices
- Use a general
catch
as a fallback:try { // ... } on SpecificException catch (e) { // Handle specific case } catch (e, s) { print('Unhandled exception: $e\nStack trace: $s'); }- Avoid bare
catch
clauses: Useon
with specific exception types to prevent silently ignoring unexpected errors[2][8].- Use
rethrow
to partially handle an exception while allowing propagation:try { misbehave(); } catch (e) { print('Partial handling'); rethrow; // Continue propagating }If the exception isn't caught anywhere in the call stack, Dart's default behavior ensures program termination to prevent undefined states[1][9].
Citations:
- 1: https://dart.dev/language/error-handling
- 2: https://dart.dev/tools/linter-rules/avoid_catches_without_on_clauses
- 3: https://www.dhiwise.com/post/dart-throw-how-to-effectively-handle-errors-and-exceptions
- 4: https://codewithandrea.com/articles/flutter-exception-handling-try-catch-result-type/
- 5: Confusing list type mismatch at runtime dart-lang/language#3185
- 6: https://www.youtube.com/watch?v=2uyMvzTr9xI
- 7: Syntax catching multiple exceptions in a single on block dart-lang/language#112
- 8: https://www.dhiwise.com/post/understanding-dart-exception-hierarchy-an-overview
- 9: https://docs.flutter.dev/app-architecture/design-patterns/result
- 10: https://groups.google.com/a/dartlang.org/g/misc/c/O1OKnYTUcoo
Action: Confirm Updated Exception Handling in FutureOrResult.asResult
The updated
asResult
method correctly uses the generic type parameterF
to catch only exceptions of that type. As verified, when an exception does not match the catch clause type, it propagates up the call stack as per Dart's standard behavior. This change is consistent with the corresponding implementation inFutureResult
and ensures that only intended exceptions are caught while all others are allowed to bubble up.
- File:
lib/src/future.dart
(Lines: 32-36)- Change Verified: The try-catch now only handles exceptions of type
F
. Exceptions of any other type will propagate, which aligns with Dart's error handling mechanism.- Outcome: The behavior is as expected and intentional. No additional changes are necessary.
27-28
:❓ Verification inconclusive
Changed asResult from getter to method with type parameter
The
asResult
has been changed from a getter to a method with a generic type parameterF extends Object
. This allows callers to specify what type of exceptions they expect, improving type safety. The implementation correctly usesResult<T, F>.success
andResult<T, F>.failure
to maintain type consistency.Make sure to update all existing calls to this method in the codebase since it's no longer a getter (they'll need parentheses now).
🏁 Script executed:
#!/bin/bash # Let's check for potential usage of asResult without parentheses in the codebase echo "Searching for potential missed updates of asResult usage (should be called with parentheses now)..." rg "asResult(?!\()" --include="*.dart" --exclude="lib/src/future.dart"Length of output: 721
Below is an updated review comment that explains the change and asks for a re-check of call sites. Since the initial search command failed due to invalid flag usage, please re-run the search with the revised command to ensure no call sites have been missed.
Action Required: Verify All AsResult Call Sites Are Updated
The
asResult
member onlib/src/future.dart
is now a method with a generic type parameter (F extends Object
) for improved type safety. Its implementation correctly usesResult<T, F>.success
for successful outcomes andResult<T, F>.failure
for errors:Future<Result<T, F>> asResult<F extends Object>() => then(Result<T, F>.success).onError<F>(Result<T, F>.failure);Since this change means all usages must now have parentheses (i.e., be invoked as a method rather than accessed as a getter), please verify that no outdated call sites remain.
To help with this, try re-running the search using this revised command (note the use of
-g
for glob patterns):If this revised search yields any results, update those call sites to use parentheses. Otherwise, confirm that all calls have been correctly updated. Once verified, please mark this review comment as resolved.
🏁 Script executed:
#!/bin/bash echo "Re-running search for asResult usage without parentheses in Dart files..." rg "asResult(?!\()" -g "*.dart" -g '!lib/src/future.dart'Length of output: 564
Action Required: Verify All
asResult
Call Sites Are UpdatedThe
asResult
member inlib/src/future.dart
has been refactored from a getter into a method with a generic type parameter (F extends Object
). This change improves type safety by allowing callers to specify the exception type. The implementation usingResult<T, F>.success
andResult<T, F>.failure
is correct:Future<Result<T, F>> asResult<F extends Object>() => then(Result<T, F>.success).onError<F>(Result<T, F>.failure);Since this change requires that all usages now invoke the method with parentheses, please verify that no outdated call sites exist.
To aid in verification, re-run the search with PCRE2 enabled (for proper support of look-ahead) using:
If this search produces any matches, update those call sites to invoke
asResult()
properly. If no results are found, confirm that all calls have been updated accordingly.
🏁 Script executed:
#!/bin/bash echo "Searching for any asResult calls without parentheses using PCRE2 regex..." rg --pcre2 "asResult(?!\()" --glob "*.dart" --glob "!lib/src/future.dart"Length of output: 1151
Action Required: Confirm All Call Sites Invoke
asResult
as a MethodThe
asResult
member inlib/src/future.dart
is now defined as a method with a generic type parameter (F extends Object
), ensuring that callers can explicitly specify their expected exception types. The updated implementation is:Future<Result<T, F>> asResult<F extends Object>() => then(Result<T, F>.success).onError<F>(Result<T, F>.failure);Since this refactoring requires callers to invoke
asResult()
with parentheses rather than accessing it as a getter, we performed a regex search (using PCRE2) to identify any instances where the method might be referenced without parentheses. The search output showed matches intest/inline_result_test.dart
—however, these occurrences appear in test description strings (e.g., in test names) rather than in actual code invocations.Please manually verify that:
- All real call sites in production code invoke
asResult
with parentheses.- The matches from test files are only part of descriptive text and that any invocations (such as
future.asResult<Exception>()
) correctly include parentheses.Once you have confirmed that there are no outdated call sites, you can mark this review comment as verified.
lib/src/fold.dart (3)
12-16
: Looks good for broadening failure type support.
The shift fromException
toF extends Object
forFailureTransformer
correctly generalizes the failure flow.
20-20
: Future extension signature update is correct.
This is consistent with the newResult<T, F>
type, maintaining a uniform interface across async and sync folds.
28-32
: Async folding logic cleanly handles typed failures.
Your usage of_Failure<F>
for the async fold scenario is spot-on. No concerns here.lib/src/run_catching.dart (6)
8-8
: Expanded generic parameter for runCatching.
Allowing any typeF extends Object
for the exception significantly enhances flexibility.
11-12
: Typed catch block is clear.
Catchingon F
ensures we only intercept the intended failure type in this usage.
20-22
: Use of generic failure type in asyncRunCatching.
This aligns well with the synchronous counterpart, enabling seamlessly typed exception handling in async workflows.
25-25
: Relevant exception catch.
Specifyingon F
again ensures only the typed failure is handled. Good practice.
38-41
: Extension method signature update is consistent.
The refactor forrunCatching<R, F>
similarly ensures typed exception coverage at call sites.
53-58
: Full async coverage for typed exceptions.
The method now correctly captures any object-based failure, matching the rest of the code.lib/src/result.dart (4)
27-30
: Typedefs now allow broader exception types.
ReplacingE extends Exception
withE extends Object
is a sound approach for domain-specific error handling.
32-35
: AsyncFailureTransformer signature aligns with the new design.
No issues with the expanded type constraint; usage remains straightforward.
46-46
: Result extension type with typed failures.
Introducing<T, F extends Object>
improves type safety and clarifies intended usage for failures.
51-51
: Factory constructor for typed failures is well-structured.
This pairs well with_Failure<F>
to store any object-based exception.lib/src/getter.dart (8)
4-4
: ResultGetter with typed failures.
Updating the extension toResultGetter<T, F extends Object>
aligns with the rest of the code.
11-11
: Exception retrieval is accurate.
Using_value is _Failure<F>
to return the typed exception ensures only the intended type is accessed.
17-17
: Stack trace handling for typed failures.
The approach remains consistent, returningnull
when no trace is present.
23-26
: Throwing typed exceptions in getOrThrow.
This preserves distinct error types while integrating seamlessly with the new generic approach.
27-36
: Flexible retrieval using getOrElse / getOrDefault.
Allowing a typedFailureTransformer
or default fallback fosters more robust failure handling.
40-40
: FutureResultGetter extension signature updated.
Consistent with the synchronous counterpart, ensures typed exceptions across async calls.
47-47
: Async exceptionOrNull.
ReturningFuture<F?>
precisely matches the typed failure approach.
60-74
: Async getters reflect typed error usage.
Methods likegetOrThrow
,getOrElse
, andgetOrDefault
forFuture<Result<T,F>>
elegantly unify async flows with typed failures.lib/src/recover.dart (7)
4-4
: Consistent Generics Implementation
The addition ofF extends Object
to this extension aligns well with the new typed failure approach, offering increased flexibility for error handling.
11-14
: Well-Structured Recovery Logic
DefiningE extends F
inrecover
enforces a more specific exception type while preserving the broader typed failure system. The check_value is _Failure<F>
is correct for validating the error type.
20-29
: Robust Error-Catching Mechanism
TherecoverCatching
method neatly wraps transformation exceptions in a newResult
, maintaining consistent typed failure behavior. Nice approach for preventing unhandled errors.
32-32
: Future Extension for Typed Failure
IntroducingFutureResultRecover<T, F extends Object>
is a valuable addition, ensuring async flows also benefit from typed error handling.
39-49
: Async Recover Respects Failure Type
The transformation logic forrecover
on aFuture<Result<T,F>>
reads clearly and follows the same pattern as the synchronous version, preserving the typed error.
54-54
: Consistent Signature for recoverCatching
UsingE extends F
here ensures transformations can remain highly specific about which error types are to be caught.
57-63
: Inline asResult Usage
Applyingtransform(...).asResult()
when a failure matchesE
is an elegant way to unify async error transformation under typed failures. Nicely done.lib/src/on_action.dart (6)
4-4
: Extension Naming Consistency
extension ResultOnActions<T, F extends Object>
clearly communicates that these helpers accommodate both a success typeT
and a typed failureF
.
9-16
: onFailure Method Maintains Typed Failure
Your check_value is _Failure<F> && _value.exception is E
correctly narrows to the exception subtype, allowing safe invocation of theFailureTransformer
.
22-27
: onSuccess Maintains Original Value
Executing a side-effect on success while returning the sameResult
ensures no functional disruption. This design is straightforward and well-implemented.
30-30
: Future Extension Intent
Bringing the same “on action” logic toFuture<Result<T, F>>
extends usability for async flows, creating a uniform API.
35-39
: Async onFailure Method
Accepting aFailureTransformer<void, E>
for asynchronous usage keeps the success/failure chain consistent. Good clarity on side effects in async contexts.
44-45
: Async onSuccess Method
The method preserves the original result and adds an action if the future resolves to a success, mirroring the synchronous counterpart neatly.test/inline_result_test.dart (31)
12-12
: Verification of Basic runCatching
This test confirms that a successfulrunCatching
call correctly setsisSuccess
. It effectively validates the new typed approach without complicating the exception type.
30-31
: Typed Failure Test
runCatching<String, Exception>
showcases the typed failure usage. The test coverage ensures that thrownCustomException
is captured as expected.
58-59
: Failure Construction
Explicitly writingResult<String, Exception>.failure(...)
clarifies the exception type in tests, solidifying the typed approach.
82-83
: StackTrace Preservation
ProvidingStackTrace.empty
ensures coverage for scenarios with an explicit stacktrace, verifying that it flows through a typed failure properly.
107-109
: FlatMap Test with Pre-Existing Failure
This test checks that a failure short-circuits the mapping function, verifying the typed error remains unaltered.
122-123
: Simple recover Usage
Verifies thatrecover
can convert a knownCustomException
into a success with a fallback value—useful coverage for typed transformations.
131-131
: runCatching with FormatException
Demonstrates typed fallback logic for different exception subtypes. The test ensures the first recover branch is matched.
145-145
: ConsistentResult<int, Exception>
Explicit success type parameter helps clarify that success and failure branches share a typed structure.
151-152
: recoverCatching for Failure
Ensures thatrecoverCatching
properly transforms a typed failure to a success, reinforcing typed exception coverage.
158-159
: Verify Thrown Exception
The transformation function deliberately throws anotherCustomException
, validating that a second failure remains typed.
167-167
: No Impact on Success
recoverCatching
on a success returns the original value, confirming that the typed approach does not interfere with success paths.
181-183
: onFailure for a Typed Exception
The side-effect logic is tested thoroughly with a typedCustomException
, verifyingonFailure
usage.
195-195
: Stacktrace Existence
Ensures the test intentionally throws an exception to confirm thatrunCatching
produces a typed failure with a stacktrace.
233-234
: Async runCatching with a Typed Throw
Demonstrates typed exception usage in async transformations. If the function throws, the result is a typed failure.
246-246
: asResult for Successful Future
Ensures synchronous success inside a future becomes a typedResult.success()
.
253-253
: Ensure Exception is Captured
Converts a failing future ofint
to a typed failure with a genericException
. Good coverage for unexpected runtime errors.
262-262
: CustomException in Future
Tests typed capturing ofCustomException
from a future.
271-271
: Non-Exception Error
Validates that non-Exception
errors cause test failures, clarifying typed constraints.
276-276
: Delayed Success
Confirms that a delayed success is wrapped as a typedResult.success()
.
286-286
: Delayed Failure
Similarly ensures that a delayed throw with anException
yields a typed failure.
302-303
: Future fold for Failure
Guarantees the fallback branch is triggered for typed failures.
344-346
: Future onFailure
Ensures that the side-effect logic is executed for a typed failure. The captured failure meetsException
constraints.
357-358
: getOrElse after Failure
Checks that typed failure can be recovered with a fallback integer.
367-368
: getOrDefault after Failure
Validates that the typed failure returns the provided default, preserving typed behavior.
374-375
: recover with Async Future
Asserts that an already-typed failure is recoverable to a success integer.
387-388
: recoverCatching for Future
Ensures the typed failure is transformable without altering success paths.
395-396
: Caught Second Failure
Validates throwing anotherCustomException
remains typed in the recovery transform.
423-423
: asResult with FutureOr
Shows thatFutureOr
values also integrate seamlessly into typed results.
429-429
: Synchronous Throw from Future
Confirmed that an immediate throw is captured asCustomException
in typed results.
442-445
: map on a Failure
Ensures the typed failure is propagated without transformation.
457-461
: mapCatching Exception
Tests that an exception thrown during transformation yields a typed failure.lib/src/transformation.dart (8)
4-4
: Generic ResultTransformation
Introducing<T, F extends Object>
broadens the range of permissible failures while retaining type safety.
10-12
: map Maintains Original Failure
If aResult
is not successful, returningResult<R, F>._(_value)
correctly preserves the original typed failure.
18-20
: mapCatching with runCatching
By wrapping the transform inrunCatching<R, F>
, any thrown exceptions convert to typed failures, ensuring graceful fallback.
27-34
: flatMap for Nested Results
If_value
is_Failure<F>
, the same typed failure is forwarded; otherwise, you apply the user-suppliedtransform
. Good short-circuit logic.
36-37
: FutureResultTransformation Extension
AllowingFuture<Result<T, F>>
transformations fosters consistent usage of typed failures in asynchronous contexts.
43-46
: map with Async
Spreads the logic of synchronous map to futures. If the underlying value is a typed failure, it remains unmodified; otherwise, it appliestransform(...)
.
52-55
: mapCatching with asyncRunCatching
Gracefully captures any thrown exception during transformation, converting it to a typed failure for uniform error handling in async code.
62-71
: flatMap for Future Result
Predicates on_Failure<F>
to re-use the existing typed failure or applytransform(...)
. The typed system remains consistent in async flows.
Summary by CodeRabbit