fix: Try.Run must not throw on whitespace exception message (#273) - #276
Merged
Conversation
Closes #273. `Try.Run*` was catching every Exception and passing `ex.Message` straight to `Result.Failure`. `Result.Failure` rejects null / empty / whitespace strings with ArgumentException — meaning `Try.Run(() => throw new X(" "))` threw ArgumentException from inside its own catch instead of returning a Failed Result. Defeats the entire point of the wrapper for any exception whose Message happens to be whitespace-only (or one thrown with the parameterless framework ctor, whose default message is culture- dependent and can be empty). Fix: private `SafeErrorMessage(Exception)` helper coerces a null / empty / whitespace `ex.Message` to `ex.GetType().Name` before handing it to `Result.Failure`. All four catch blocks (Run, Run<T>, RunAsync, RunAsync<T>) route through it. Regression tests in `TryRunWhitespaceMessageTests.cs` cover both the whitespace-coerce-to-type-name path and the real-message preservation path across every overload. Discovered by the FsCheck fuzz property added in PR #274. Once this ships and #274 lands, the `string.IsNullOrWhiteSpace` skip in that PR's Fuzz_Try_Run_of_throwing_action_carries_message + Try_Run_of_throwing_action_carries_message_round_trip properties can be removed as a follow-up so the fuzz exercises the whole space including whitespace inputs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This was referenced Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #273.
Bug
Try.Run*catches everyExceptionand passesex.Messagestraight toResult.Failure.Result.Failurerejects null / empty / whitespace withArgumentException— soTry.Run(() => throw new X(" "))throwsArgumentExceptionfrom inside its own catch instead of returningFailed. Defeats the whole point of the wrapper.Fix
Private
SafeErrorMessage(Exception)helper coerces null / empty / whitespaceex.Messagetoex.GetType().Name. Every catch block (Run(Action),Run<T>,RunAsync(Action),RunAsync<T>) routes through it.Regression tests
TryRunWhitespaceMessageTests.cscovers:""," ","\t","\n"on every overload → returnsFailed,ErrorMessage == nameof(InvalidOperationException)Discovery
FsCheck property
Fuzz_Try_Run_of_throwing_action_carries_messagein PR #274 falsified this on iteration 19. Filed as #273, tightened the property with a whitespace-skip guard so #274 could ship clean; this PR is the real fix. Once both land, a follow-up removes the skip so the fuzz exercises the whole message space.Stacked-PR base
Base =
vNext.Local run
dotnet test -c Release -f net10.0→ 102/102 pass (11 new whitespace-regression tests + the existing 91). Zero warnings.Test plan
IsNullOrWhiteSpaceskip in FuzzTests / PropertyTests