fix: wrap repo service errors in UserError for cleaner output#88
Merged
github-actions[bot] merged 1 commit intomainfrom Jan 31, 2026
Merged
Conversation
- Wrap "repository already initialized" and "not a swamp repository" errors in UserError to suppress stack traces - These are expected user-facing conditions where the stack trace is noise ### Test plan - deno check passes - deno run test src/domain/repo/ passes
There was a problem hiding this comment.
Review Summary
This PR correctly wraps repository initialization errors in UserError to suppress stack traces for expected user-facing conditions. The changes align well with the established error handling pattern in this codebase.
Analysis
Code Quality: ✅
- Clean, minimal change that follows existing patterns
UserErroris already used throughout the codebase (16 files) for similar purposes- The error handling in
src/presentation/output/error_output.tsx:32-33properly detectsUserErrorinstances and suppresses stack traces
TypeScript Compliance: ✅
- No
anytypes - Uses named imports correctly
- Type-safe implementation
Test Coverage: ✅
- Existing tests at
repo_service_test.ts:115-129andrepo_service_test.ts:183-194will continue to pass becauseUserErrorextendsError - The
assertRejectscalls check forErrortype and error message content, both of which are preserved
DDD Principles: ✅
UserErroris appropriately defined in the domain layer (src/domain/errors.ts)- The separation between internal errors (show stack) and user-facing errors (no stack) is a valid domain concept
Security: ✅
- No security concerns - this change actually improves UX by reducing noise in error output
Suggestions (Non-blocking)
- Consider stronger test assertions: The tests could be updated to assert
UserErrorspecifically instead ofErrorto document the intended behavior more explicitly. However, this is optional since the current tests correctly verify the error messages.
LGTM!
stack72
added a commit
that referenced
this pull request
Apr 13, 2026
forEach.in is evaluated synchronously, but async CEL functions
(data.latest, data.findByTag, data.findBySpec, data.query) return
Promises that coerceBigInts silently converts to {}, causing forEach
to expand zero steps with no error. Add Promise detection in
CelEvaluator.evaluate before coerceBigInts runs, and wrap the error
in expandForEachSteps and libswamp evaluate.ts with a UserError that
names the expression and points to the nested workflow workaround.
Closes: swamp-club #88
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
9 tasks
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.
Test plan