Harmonise ReadBlob: rename Source→Input, add error handling (v3.0.0)#118
Open
Harmonise ReadBlob: rename Source→Input, add error handling (v3.0.0)#118
Conversation
…uccess/Error result
- Renamed Source class/tab to Input (breaking: update task parameter references)
- Added Options.ThrowErrorOnFailure (default true) and Options.ErrorMessageOnFailure
- Added Result.Success (bool) and Result.Error { Message, AdditionalInfo }
- Created Helpers/ErrorHandler.cs following standard Frends error handling pattern
- Created Definitions/Error.cs for structured error details
- Bumped version to 3.0.0 (breaking changes)
- Updated tests: renamed Source→Input, added error handling test cases
- Updated CHANGELOG.md with upgrade instructions
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThe PR introduces breaking changes to the ReadBlob task: renames the Changes
Sequence DiagramsequenceDiagram
participant Caller
participant ReadBlob as ReadBlob.cs
participant ErrorHandler as ErrorHandler
participant Result as Result
Caller->>ReadBlob: Call ReadBlob(input, connection, options)
activate ReadBlob
alt Success Path
ReadBlob->>ReadBlob: Validate & download blob
ReadBlob->>Result: Create Result with Success=true, Content, Error=null
Result-->>ReadBlob: Result object
ReadBlob-->>Caller: Return Result
else Exception Path
ReadBlob->>ReadBlob: Exception caught
ReadBlob->>ErrorHandler: Handle(exception, ThrowErrorOnFailure, ErrorMessageOnFailure)
activate ErrorHandler
alt ThrowErrorOnFailure = true
ErrorHandler->>ErrorHandler: Create Exception with custom message
ErrorHandler-->>ReadBlob: Throw Exception
ReadBlob-->>Caller: Exception propagates
else ThrowErrorOnFailure = false
ErrorHandler->>Result: Create Result with Success=false, Error populated
ErrorHandler-->>ReadBlob: Return Result
ReadBlob-->>Caller: Return Result with Error details
end
deactivate ErrorHandler
end
deactivate ReadBlob
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
Frends.AzureBlobStorage.ReadBlob/CHANGELOG.md (1)
5-11: Categorize new items underAddedrather thanChanged.Per Keep a Changelog, new features/options (
Options.ThrowErrorOnFailure,Options.ErrorMessageOnFailure,Result.Success,Result.Error) belong under### Added, while theSource→Inputrename stays under### Changed. Current structure lumps everything underChanged.📝 Proposed structure
### Changed - [Breaking] Renamed `Source` tab to `Input` tab; move task parameter references from `Source` to `Input` when upgrading. + +### Added + - Added `Options.ThrowErrorOnFailure` (default: `true`) — when `false`, errors are returned in the result instead of thrown. - Added `Options.ErrorMessageOnFailure` — custom error message prefix for thrown exceptions or returned errors. - Added `Result.Success` (bool) — `true` on success, `false` on failure. - Added `Result.Error` (object `{ string Message, object AdditionalInfo }`) — populated on failure when `ThrowErrorOnFailure` is `false`.As per coding guidelines, "Validate format against Keep a Changelog".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.ReadBlob/CHANGELOG.md` around lines 5 - 11, Split the changelog entries so that the rename of `Source` → `Input` remains under the "### Changed" section, and move the new API/features (`Options.ThrowErrorOnFailure`, `Options.ErrorMessageOnFailure`, `Result.Success`, `Result.Error`) into a new "### Added" section; update the headings and ordering in CHANGELOG.md to follow Keep a Changelog format so additions are under "Added" and only breaking/rename items remain under "Changed".Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/Helpers/ErrorHandler.cs (1)
10-16: Avoid throwing the baseSystem.Exceptiontype.Throwing
new Exception(...)is discouraged (analyzer CA2201) because callers cannot catch it selectively without also catching every other failure. ConsiderInvalidOperationException(or a task-specific exception) and preserve the original asInnerException— which you already do.♻️ Suggested change
- if (throwOnFailure) - { - if (string.IsNullOrEmpty(errorMessageOnFailure)) - throw new Exception(exception.Message, exception); - - throw new Exception(errorMessageOnFailure, exception); - } + if (throwOnFailure) + { + var message = string.IsNullOrEmpty(errorMessageOnFailure) + ? exception.Message + : errorMessageOnFailure; + throw new InvalidOperationException(message, exception); + }Note: this changes the thrown exception type, so any tests asserting on
Exceptionspecifically should be reviewed accordingly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/Helpers/ErrorHandler.cs` around lines 10 - 16, The code currently throws System.Exception in the error handling branch (the throw new Exception(...) lines in ErrorHandler.cs); replace those with a more specific exception type such as InvalidOperationException (or a task-specific custom exception) and preserve the original exception as the InnerException (i.e., throw new InvalidOperationException(errorMessageOnFailure ?? exception.Message, exception)); update any tests that assert on Exception to expect the new exception type if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Frends.AzureBlobStorage.ReadBlob/CHANGELOG.md`:
- Around line 3-17: The CHANGELOG has a date mismatch: the 3.0.0 heading shows
2026-04-17 which is earlier than the 2.0.0 entry dated 2026-04-26; update the
3.0.0 release date (the heading "## [3.0.0] - 2026-04-17") to the correct
chronological date (or correct 2.0.0 if that one was wrong) so release order is
chronological, preserving the 3.0.0 changelist text and the 2.0.0 header.
In
`@Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob.Tests/UnitTests.cs`:
- Around line 157-160: Change the test to assert the specific
ValidationException and use the constraint form for message checking: replace
Assert.ThrowsAsync<Exception>(() => AzureBlobStorage.ReadBlob(...)) with
Assert.ThrowsAsync<ValidationException>(() => AzureBlobStorage.ReadBlob(...))
and replace the boolean message assertion
Assert.That(ex.Message.Contains("SasToken is required."), ex.Message) with
Assert.That(ex.Message, Does.Contain("SasToken is required.")); apply the same
two changes for the other failing test that checks "ConnectionString is
required." so both tests use ValidationException and Does.Contain for message
assertions.
In
`@Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/Helpers/ErrorHandler.cs`:
- Around line 8-28: The ErrorHandler.Handle method currently converts
OperationCanceledException/TaskCanceledException into generic failures; update
Handle (or the caller in ReadBlob) to let cancellation propagate: check for
exception is OperationCanceledException or TaskCanceledException at the start of
ErrorHandler.Handle and rethrow it immediately (regardless of throwOnFailure),
or alternatively add an exception filter in ReadBlob's catch to rethrow when ex
is a cancellation exception before calling ErrorHandler.Handle; reference
ErrorHandler.Handle and ReadBlob's catch/throwOnFailure so the change is applied
to the correct locations.
---
Nitpick comments:
In `@Frends.AzureBlobStorage.ReadBlob/CHANGELOG.md`:
- Around line 5-11: Split the changelog entries so that the rename of `Source` →
`Input` remains under the "### Changed" section, and move the new API/features
(`Options.ThrowErrorOnFailure`, `Options.ErrorMessageOnFailure`,
`Result.Success`, `Result.Error`) into a new "### Added" section; update the
headings and ordering in CHANGELOG.md to follow Keep a Changelog format so
additions are under "Added" and only breaking/rename items remain under
"Changed".
In
`@Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/Helpers/ErrorHandler.cs`:
- Around line 10-16: The code currently throws System.Exception in the error
handling branch (the throw new Exception(...) lines in ErrorHandler.cs); replace
those with a more specific exception type such as InvalidOperationException (or
a task-specific custom exception) and preserve the original exception as the
InnerException (i.e., throw new InvalidOperationException(errorMessageOnFailure
?? exception.Message, exception)); update any tests that assert on Exception to
expect the new exception type if needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3561b607-508f-4f5d-8034-d3cd4312f48d
📒 Files selected for processing (9)
Frends.AzureBlobStorage.ReadBlob/CHANGELOG.mdFrends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob.Tests/UnitTests.csFrends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/Definitions/Error.csFrends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/Definitions/Input.csFrends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/Definitions/Options.csFrends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/Definitions/Result.csFrends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob.csprojFrends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/Helpers/ErrorHandler.csFrends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/ReadBlob.cs
| - Added `Result.Success` (bool) — `true` on success, `false` on failure. | ||
| - Added `Result.Error` (object `{ string Message, object AdditionalInfo }`) — populated on failure when `ThrowErrorOnFailure` is `false`. | ||
|
|
||
| #### Upgrade instructions |
Contributor
There was a problem hiding this comment.
remove update notes from changelog
| ### Changed | ||
|
|
||
| - [Breaking] Renamed `Source` tab to `Input` tab; move task parameter references from `Source` to `Input` when upgrading. | ||
| - Added `Options.ThrowErrorOnFailure` (default: `true`) — when `false`, errors are returned in the result instead of thrown. |
Contributor
There was a problem hiding this comment.
move added to added header
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
3 participants
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.
Summary
Sourcetab toInput— update task parameter references fromSourcetoInputwhen upgrading;ContainerNameandBlobNameproperties are unchanged.Options.ThrowErrorOnFailure(default:true) andOptions.ErrorMessageOnFailureto control error behaviour.Result.Success(bool) andResult.Error { Message, AdditionalInfo }to the result object.Helpers/ErrorHandler.csandDefinitions/Error.csfollowing the standard Frends error handling pattern.Resolves FSPW-397.
Test plan
Inputand assertSuccess = trueThrowErrorOnFailure = falsereturning error in resultThrowErrorOnFailure = truewith custom message throwing correct exceptiondotnet build)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Breaking Changes
Sourcetab is nowInput. Update existing task configurations accordingly.New Features
ThrowErrorOnFailureoption (default: true) to control whether errors are thrown or returned in results.ErrorMessageOnFailureoption to customize error message prefixes.Successstatus and structured error details when exceptions are not thrown.