Skip to content

Conversation

@adelrodriguez
Copy link
Collaborator

@adelrodriguez adelrodriguez commented Jan 30, 2026

Greptile Overview

Greptile Summary

This PR refactors the error handling system by replacing faultier with better-result, transitioning from Fault-based errors to TaggedError classes.

Key Changes

  • New Error Architecture: Created domain-specific error classes (UnauthenticatedError, UnauthorizedError, SendEmailError, etc.) using TaggedError from better-result
  • Simplified Error Handling: Removed Fault's serialization, wrapping, and description methods in favor of direct error instantiation
  • Package Updates: Replaced faultier 1.1.0 with better-result 2.6.0
  • Cleanup: Removed Fault-specific utilities (HTTPFault, TRPCFault) and serialization adapters

Areas Requiring Attention

  • Error Logging: The API error handler (apps/api/src/routes/index.ts:58) now only logs error.message instead of the previously captured structured metadata (cause, context, debug info, tags). This may reduce debugging capabilities for production issues.

Confidence Score: 4/5

  • This PR is generally safe to merge with one area needing attention around error logging
  • The refactoring is well-executed with consistent patterns across all changed files. Error classes are properly defined with appropriate metadata. The score is 4 instead of 5 due to the loss of structured error logging in the API error handler, which could impact debugging production issues.
  • Pay attention to apps/api/src/routes/index.ts - verify that simplified error logging meets observability requirements

Important Files Changed

Filename Overview
packages/error/src/auth.ts Introduced new error classes using TaggedError from better-result for authentication errors
packages/error/src/email.ts Created email-specific error classes with metadata for send and batch operations
packages/error/src/utils.ts Replaced Fault-based utility errors with TaggedError classes for duration parsing and assertions
packages/error/src/index.ts New entry point aggregating all error types into unified AppError type
apps/api/src/routes/index.ts Simplified error handler by removing Fault-specific logic, now logs generic error messages
apps/app/src/shared/server/middleware.ts Replaced Fault error creation with direct UnauthenticatedError and UnauthorizedError instantiation
packages/email/src/client.ts Replaced Fault.wrap() with direct error class instantiation for email send failures

Sequence Diagram

sequenceDiagram
    participant Client
    participant API/Middleware
    participant Service
    participant ErrorClass as TaggedError (better-result)

    Note over Client,ErrorClass: Authentication Flow
    Client->>API/Middleware: Request (unauthenticated)
    API/Middleware->>API/Middleware: Validate session/identity
    API/Middleware->>ErrorClass: new UnauthenticatedError()
    ErrorClass-->>Client: Throw error

    Note over Client,ErrorClass: Authorization Flow
    Client->>API/Middleware: Request (non-admin user)
    API/Middleware->>API/Middleware: Check user role
    API/Middleware->>ErrorClass: new UnauthorizedError({userId})
    ErrorClass-->>Client: Throw error with userId

    Note over Client,ErrorClass: Email Send Flow
    Client->>Service: sendEmail(body, params)
    Service->>Service: Resend API call
    Service->>Service: API returns error
    Service->>ErrorClass: new SendEmailError({emails, from, subject, text})
    ErrorClass-->>Client: Throw error with metadata

    Note over Client,ErrorClass: Duration Parse Flow
    Client->>Service: parse(invalidString)
    Service->>Service: Validate input
    Service->>ErrorClass: new InvalidDurationParseInputError({value})
    ErrorClass-->>Client: Throw error with value

    Note over API/Middleware,ErrorClass: Error Handling (apps/api)
    API/Middleware->>API/Middleware: onError handler catches error
    API/Middleware->>API/Middleware: Check if HTTPException
    alt is HTTPException
        API/Middleware-->>Client: Return exception response
    else other error
        API/Middleware->>API/Middleware: Log error.message
        API/Middleware->>API/Middleware: captureException(error)
        API/Middleware-->>Client: Return 500 Internal Server Error
    end
Loading

@adelrodriguez adelrodriguez marked this pull request as ready for review January 30, 2026 04:34
@coderabbitai
Copy link

coderabbitai bot commented Jan 30, 2026

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

  • 🔍 Trigger a full review
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor_replace_faultier_with_better-result_for_error_handling

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Collaborator Author

adelrodriguez commented Jan 30, 2026

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

return error.getResponse()
}

c.var.logger.error(error.message)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lost detailed error information. Previously logged structured data including cause, context, debug info, and tags from Fault errors. Now only logs error.message which may not capture full error context for debugging.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/api/src/routes/index.ts
Line: 58:58

Comment:
Lost detailed error information. Previously logged structured data including cause, context, debug info, and tags from Fault errors. Now only logs `error.message` which may not capture full error context for debugging.

How can I resolve this? If you propose a fix, please make it concise.

@adelrodriguez adelrodriguez merged commit 1f7daa3 into main Jan 30, 2026
9 checks passed
@adelrodriguez adelrodriguez deleted the refactor_replace_faultier_with_better-result_for_error_handling branch January 30, 2026 04:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants