Remove persistence error constructor wrappers#3398
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
ApprovabilityVerdict: Approved Mechanical refactoring that replaces curried error constructor wrappers with direct constructor/static method calls. Changes maintain backwards compatibility, add unit tests, and include a privacy improvement to avoid leaking payload data in error diagnostics. You can customize Macroscope's approvability policy. Learn more. |
Co-authored-by: codex <codex@users.noreply.github.com>
52640c0 to
3a877d2
Compare
Dismissing prior approval to re-evaluate 3a877d2
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate dd36e26
Removes the valueless persistence error constructor wrappers and constructs SQL errors directly where operation and cause are known. Generic duplicated detail text is dropped; specific detail remains supported. SchemaError conversion moves to the valueful PersistenceDecodeError.fromSchemaError mapper.
Validation:
Note
Low Risk
Localized persistence error construction and logging shape changes; no auth or data-path logic changes, with tests covering message and diagnostic behavior.
Overview
Persistence repositories now build
PersistenceSqlErrorandPersistenceDecodeErrordirectly instead of going through thintoPersistence*wrappers.PersistenceSqlError.detailis optional, so messages likeSQL error in <operation>no longer append a generic “Failed to execute …” suffix when only operation and cause are known.Schema failures are mapped via
PersistenceDecodeError.fromSchemaError(), which summarizes issues with a smallsummarizeSchemaIssuehelper (issue tags only) rather than the full default formatter—so rejected row/payload values are not copied intoissueormessage.toPersistenceDecodeCauseErroris removed; legacytoPersistenceSqlError/toPersistenceDecodeErrorremain for orchestration/projection call sites. New tests lock in SQL message shape and decode diagnostics that exclude sensitive rejected data.Reviewed by Cursor Bugbot for commit dd36e26. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Remove persistence error constructor wrapper functions in favor of direct error instantiation
toPersistenceDecodeErrorandtoPersistenceSqlErrorhelper calls with direct construction viaPersistenceDecodeError.fromSchemaError(operation, cause)andnew PersistenceSqlError({ operation, cause })across persistence repositories.summarizeSchemaIssuein Errors.ts to produce compact issue strings fromSchemaIssue.Issue, avoiding rejected payload values leaking into error diagnostics.PersistenceSqlError.detailoptional; messages now read'SQL error in <operation>'when no detail is provided (previously always included a detail string).SchemaIssueformatter output; SQL errors constructed via the new path omit the'Failed to execute <operation>'detail string.Macroscope summarized dd36e26.