chore(rivetkit): RIVET_EXPOSE_ERRORS

[NathanFlurry]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
rivet-site	Ready	Preview, Comment	Dec 23, 2025 10:48pm
rivetkit-serverless	Error		Dec 23, 2025 10:48pm

2 Skipped Deployments

Project	Deployment	Review	Updated (UTC)
rivet-cloud	Ignored		Dec 23, 2025 10:48pm
rivet-inspector	Ignored	Preview	Dec 23, 2025 10:48pm

[NathanFlurry]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• chore(docs): document abort signal #3673
• chore(rivetkit): clean up logging configuration #3672
• chore(rivetkit): RIVET_EXPOSE_ERRORS #3671 👈 (View in Graphite)
• chore(rivetkit): specialize context types for better compiler errors #3669
• fix(rivetkit): fix c.client not respecting correct origin #3668
• fix(rivetkit): allow providing no args to createRivetKit #3667
• main

---

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

PR Review: RIVET_EXPOSE_ERRORS Implementation

Summary

This PR implements a new environment variable RIVET_EXPOSE_ERRORS to control whether internal error details are exposed to clients. The implementation looks solid with good documentation, but there are a few areas for improvement.

---

✅ Strengths

1. Good security defaults: Errors are not exposed by default, requiring explicit opt-in
2. Clear documentation: The website/src/content/docs/actors/errors.mdx addition provides excellent guidance on when and how to use this feature
3. Consistent implementation: The logic is consistently applied in both protocol handlers (old.ts) and router endpoints
4. Development-friendly: Auto-enabling in NODE_ENV=development improves DX

---

🔍 Code Quality Issues

1. Duplicated Logic (Medium Priority)

The same environment check is duplicated in two places:

• rivetkit-typescript/packages/rivetkit/src/actor/protocol/old.ts:291-292
• rivetkit-typescript/packages/rivetkit/src/actor/router-endpoints.ts:195-196

Recommendation: Extract this into a shared helper function to maintain DRY principles:

// In src/utils.ts or src/common/utils.ts
export function shouldExposeErrors(): boolean {
  return (
    getEnvUniversal("RIVET_EXPOSE_ERRORS") === "1" ||
    getEnvUniversal("NODE_ENV") === "development"
  );
}

Then use it consistently:

// In old.ts
deconstructError(error, actor.rLog, {...}, shouldExposeErrors());

// In router-endpoints.ts
export function getRequestExposeInternalError(_req: Request): boolean {
  return shouldExposeErrors();
}

2. Function Documentation vs Implementation Mismatch (Low Priority)

In router-endpoints.ts:189-192, the comment says:

"Determines whether internal errors should be exposed to the client."

But the function signature includes an unused _req: Request parameter (note the underscore prefix). This suggests future plans to make decisions per-request, but that's not implemented.

Recommendation: Either:

• Remove the unused parameter if per-request exposure isn't planned
• Update the documentation to explain why the parameter exists but isn't used yet

---

🧪 Testing Concerns

Missing test coverage for the new functionality. Consider adding tests for:

1. Environment variable behavior:

   describe('error exposure', () => {
     it('should expose errors when RIVET_EXPOSE_ERRORS=1', () => {
       process.env.RIVET_EXPOSE_ERRORS = '1';
       expect(getRequestExposeInternalError(mockReq)).toBe(true);
     });
     
     it('should expose errors when NODE_ENV=development', () => {
       process.env.NODE_ENV = 'development';
       expect(getRequestExposeInternalError(mockReq)).toBe(true);
     });
     
     it('should not expose errors by default', () => {
       delete process.env.RIVET_EXPOSE_ERRORS;
       process.env.NODE_ENV = 'production';
       expect(getRequestExposeInternalError(mockReq)).toBe(false);
     });
   });

2. Integration tests verifying that actual error messages change based on the flag

---

📝 Documentation Review

The documentation in website/src/content/docs/actors/errors.mdx is excellent! It:

• ✅ Clearly warns about production usage
• ✅ Explains when each mode is activated
• ✅ Provides concrete examples
• ✅ Emphasizes server-side logging as the primary debugging approach

Minor suggestion: Consider adding a warning box/callout around the "Warning: Only enable error exposure in development" message to make it more prominent.

---

🔒 Security Assessment

Good security posture:

• ✅ Secure by default (errors masked in production)
• ✅ Explicit opt-in required
• ✅ Documentation emphasizes the security implications
• ✅ Auto-enable only in development environment

No security concerns identified.

---

⚡ Performance Considerations

• ✅ Minimal performance impact (simple boolean checks)
• ✅ Environment variables read at decision time (no caching issues)
• ⚠️ Minor: The environment variable is read on every error. Consider caching if this becomes a hotpath, though it's unlikely to matter in practice.

---

📋 Recommendations Summary

Must Fix:

• None (code is functional)

Should Fix:

• Extract duplicated environment check logic into shared helper
• Add test coverage for the new functionality

Nice to Have:

• Clarify or remove unused _req parameter in getRequestExposeInternalError
• Consider adding a prominent warning callout in docs
• Consider caching environment variable reads if this becomes a performance concern

---

✨ Overall Assessment

This is a solid implementation of a useful development feature. The code works correctly, has sensible defaults, and includes good documentation. The main improvements are around code organization (DRY) and test coverage.

Recommendation: ✅ Approve with minor suggestions

The duplicated logic and missing tests should be addressed before or shortly after merge, but they don't block the PR from being merged.