Skip to content

Conversation

@NathanFlurry
Copy link
Member

No description provided.

@vercel
Copy link

vercel bot commented Nov 24, 2025

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

Project Deployment Preview Comments Updated (UTC)
rivet-inspector Error Error Nov 24, 2025 8:08am
rivet-site Error Error Nov 24, 2025 8:08am
rivetkit-serverless Error Error Nov 24, 2025 8:08am
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
rivet-cloud Ignored Ignored Nov 24, 2025 8:08am

@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 24, 2025

Merge activity

  • Nov 24, 8:08 AM UTC: NathanFlurry added this pull request to the Graphite merge queue.
  • Nov 24, 8:08 AM UTC: CI is running for this pull request on a draft pull request (#3535) due to your merge queue CI optimization settings.
  • Nov 24, 8:09 AM UTC: Merged by the Graphite merge queue via draft PR: #3535.

Copy link
Member Author

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


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
Copy link

claude bot commented Nov 24, 2025

PR Review: docs: update docs

Summary

This PR contains a significant documentation update with 57 files changed, including:

  • Version bump from 2.0.24 to 2.0.25-rc.1 across Cargo.lock and API specs
  • Major documentation restructuring and improvements
  • URL path updates from /docs/deploy/ to /docs/connect/
  • Removal of Unauthorized and Forbidden error classes from errors.ts
  • Addition of new gateway endpoints to OpenAPI spec
  • New documentation pages for design patterns, destroy lifecycle, request handler, and websocket handler

Code Quality & Best Practices

✅ Positives

  1. OpenAPI Enhancement (rivetkit-typescript/packages/rivetkit/scripts/dump-openapi.ts):

    • Good use of Zod for schema validation
    • Clear documentation comment explaining why manual injection is needed for actor routes
    • Proper cleanup of $schema properties from generated schemas
  2. Documentation Structure:

    • Comprehensive sitemap reorganization (website/src/sitemap/mod.ts)
    • Better categorization with collapsible sections
    • Clear navigation hierarchy
  3. URL Consistency:

    • Consistent migration from /docs/deploy/ to /docs/connect/ across README.md and frontend components

Potential Issues & Concerns

⚠️ Breaking Change - Removed Error Classes

File: rivetkit-typescript/packages/rivetkit/src/actor/errors.ts

The Unauthorized and Forbidden error classes were removed:

// REMOVED:
export class Unauthorized extends ActorError { ... }
export class Forbidden extends ActorError { ... }

Concerns:

  1. Breaking change for consumers: Any code importing Unauthorized or Forbidden from rivetkit will break at compile time
  2. Missing migration path: The documentation now suggests using UserError with codes like "forbidden", but there's no explicit migration guide
  3. Status code difference: The removed classes had explicit statusCode = 401 and statusCode = 403, while UserError defaults to statusCode = 400

Recommendation:

  • Consider adding re-exports that wrap UserError for backwards compatibility, or
  • Document this as a breaking change in release notes
  • Verify no internal code depends on these classes

⚠️ OpenAPI Schema Considerations

File: rivetkit-openapi/openapi.json

  1. Missing error schemas: The new gateway endpoints only define 200/400/500 responses without detailed error schemas
  2. Path parameter validation: The {path} parameter in /gateway/{actorId}/request/{path} might need additional documentation about encoding requirements for paths with slashes

⚠️ Type Safety in OpenAPI Script

File: rivetkit-typescript/packages/rivetkit/scripts/dump-openapi.ts:104

function injectActorRouter(openApiDoc: any) {

Using any loses type safety. Consider using the OpenAPI types from the library being used.


Documentation Review

✅ Well Done

  1. Authentication docs (website/src/content/docs/actors/authentication.mdx):

    • Clear explanation of when authentication is needed
    • Good examples for JWT, external providers, RBAC, and rate limiting
    • Proper use of tabs for Rivet Cloud vs Self-Hosted scenarios
  2. New pages:

    • design-patterns.mdx - Useful patterns documentation
    • destroy.mdx - Clear actor destruction lifecycle docs
    • request-handler.mdx and websocket-handler.mdx - Low-level handler documentation

Minor Documentation Issues

  1. Typo in error class (errors.ts:300):

    export class RequestHandlerNotDfeined extends ActorError {
    //                        ^^^^^^^^ typo: "Dfeined" should be "Defined"

    (Note: This is pre-existing, not introduced by this PR, but worth fixing)

  2. Removed content verification: Large sections were removed from fetch-and-websocket-handler.mdx (583 lines deleted). Ensure this content has been properly migrated to the new split pages.


Performance Considerations

No significant performance concerns. The changes are primarily documentation and version bumps.


Security Concerns

  1. Removed auth errors: The removal of explicit Unauthorized (401) and Forbidden (403) errors might lead developers to use generic UserError with 400 status codes, which could affect:
    • API clients expecting standard HTTP auth status codes
    • Security monitoring tools that track 401/403 responses

Recommendation: Document best practices for authentication errors, suggesting appropriate status codes.


Test Coverage

No test changes are included in this PR. The documentation and OpenAPI changes should be validated to ensure:

  • Links in documentation are not broken
  • OpenAPI spec is valid (consider running an OpenAPI validator)
  • Any examples in documentation still compile/work

Recommendations

  1. Add migration notes for the removed Unauthorized/Forbidden classes
  2. Fix the typo in RequestHandlerNotDfeinedRequestHandlerNotDefined
  3. Consider adding explicit 401/403 status code guidance in auth documentation
  4. Validate the OpenAPI spec with a standard validator
  5. Verify all documentation links work correctly after the /deploy//connect/ migration

Overall, this is a solid documentation update. The main concerns are around the breaking change with removed error classes and ensuring proper migration guidance for users upgrading to this version.

@graphite-app graphite-app bot closed this Nov 24, 2025
@graphite-app graphite-app bot deleted the 11-22-docs_update_docs branch November 24, 2025 08:09
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