Skip to content

fix(server): avoid opaque path in URL for better compatibility#1046

Merged
dinwwwh merged 2 commits into
mainfrom
unnoq/issue1043
Sep 29, 2025
Merged

fix(server): avoid opaque path in URL for better compatibility#1046
dinwwwh merged 2 commits into
mainfrom
unnoq/issue1043

Conversation

@dinwwwh
Copy link
Copy Markdown
Member

@dinwwwh dinwwwh commented Sep 29, 2025

Fixes: #1043

Summary by CodeRabbit

  • Bug Fixes
    • Standardized ORPC URL format to include an explicit host (orpc://localhost), improving URL normalization and consistent request routing across clients and servers.
  • Tests
    • Updated expectations to the new URL format and added coverage for additional ORPC URL variants to strengthen parsing and validation.

@vercel
Copy link
Copy Markdown

vercel Bot commented Sep 29, 2025

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

Project Deployment Preview Comments Updated (UTC)
orpc Ready Ready Preview Comment Sep 29, 2025 3:44am

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Sep 29, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Updated ORPC origin handling across client links, server tests, and the standard-server-peer codec to use an authority-bearing origin (orpc://localhost) instead of the authority-less orpc:/. Tests and codec serialization/decoding were adjusted to normalize and round-trip URLs with that origin.

Changes

Cohort / File(s) Summary
Client adapters: constructor default URL
packages/client/src/adapters/message-port/rpc-link.ts, packages/client/src/adapters/websocket/rpc-link.ts
Change super() base URL from orpc:/ to orpc://localhost.
Client adapter tests: expected request URL
packages/client/src/adapters/message-port/rpc-link.test.ts, packages/client/src/adapters/websocket/rpc-link.test.ts
Update expected payload URL from orpc:/ping to orpc://localhost/ping.
Server adapter tests: input/expected request URL
packages/server/src/adapters/.../rpc-handler.test.ts (bun-ws, crossws, message-port, websocket, ws)
Adjust test message URL from orpc:/ping to orpc://localhost/ping.
Standard server peer codec: URL normalization
packages/standard-server-peer/src/codec.ts
Introduce SHORTABLE_ORIGIN = 'orpc://localhost' and matcher; encode now strips orpc://localhost/ when serializing u; decode prefixes u with orpc://localhost when u starts with /, otherwise uses u as-is.
Codec tests: additional URL cases
packages/standard-server-peer/src/codec.test.ts
Add parameterized test cases for orpc://localhost/... and orpc:///localhost/... URL variants to coverage.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant L as RPCLink (client)
  participant T as Transport (WS / MessagePort)
  participant S as StandardServerPeer (codec)

  Note right of L: base origin = orpc://localhost
  C->>L: request(path="/ping", ...)
  L->>L: Build URL using base orpc://localhost
  L->>T: Send payload { u: "/ping", method, headers, body }
  T->>S: Deliver message
  S->>S: if u startsWith "/" → prefix orpc://localhost\nelse use u
  S-->>T: Route decoded request to handler
  T-->>C: Response
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • unnoq/orpc#1041 — Adjusts URL normalization/origin handling and addresses opaque-path behavior for custom schemes; strongly related to these changes.

Poem

A rabbit skitters to localhost's door,
Two slashes steady where one was before.
Paths find their way, no more lost ping-song,
I twitch my nose and hop the whole day long. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The current title specifies a server‐side fix for opaque URL paths, but the changeset also updates client adapters and the standard‐server‐peer codec to use a non‐opaque origin (“orpc://localhost”) and adjusts tests accordingly, so it does not capture the full scope of the update across both client and server libraries. The title therefore only partially relates to the main change and is too narrow in context. Please revise the title to reflect that this PR fixes opaque‐URL handling across both client and server components—for example, “fix(rpc): use non-opaque orpc://localhost origin for correct path encoding in client and server.”
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues Check ✅ Passed The pull request addresses issue #1043 by changing client-side RPCLink adapters to initialize with the non-opaque origin “orpc://localhost,” updating the standard-server-peer codec to conditionally prepend that origin for relative paths, and aligning all transport tests to expect full orpc://localhost URLs. These modifications ensure pathname assignment is no longer a no-op for custom schemes and that RPC requests include the intended path, fulfilling the linked issue’s requirement for correct URL construction in both clients and servers.
Out of Scope Changes Check ✅ Passed All code and test updates focus exclusively on replacing opaque “orpc:/” origins with “orpc://localhost” across client adapters, the standard-server-peer codec, and transport tests; there are no unrelated changes or new features beyond the objectives of issue #1043.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch unnoq/issue1043

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6d92317 and 17333c7.

📒 Files selected for processing (2)
  • packages/client/src/adapters/websocket/rpc-link.test.ts (2 hunks)
  • packages/standard-server-peer/src/codec.ts (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/client/src/adapters/websocket/rpc-link.test.ts
  • packages/standard-server-peer/src/codec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: lint
  • GitHub Check: publish-commit

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.

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @unnoq, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a compatibility issue related to how URLs are constructed and parsed within the system. By transitioning from opaque orpc:/ paths to a more explicit orpc://localhost format, the change ensures consistent and robust URL handling across client and server components, resolving a known bug and enhancing overall system reliability.

Highlights

  • URL Format Standardization: The pull request standardizes the internal URL format used across various RPC links and handlers from orpc:/ to orpc://localhost to improve compatibility and avoid issues with opaque paths.
  • Client-side RPC Link Updates: The RPCLink constructors for both MessagePort and WebSocket adapters have been updated to use orpc://localhost as the base URL.
  • Server-side RPC Handler Test Updates: All server-side RPC handler tests (Bun-WS, CrossWS, MessagePort, WebSocket, WS) have been modified to reflect the new orpc://localhost/ping URL format in their request messages.
  • Codec Logic Refinement: The codec.ts file now correctly handles the new URL format during encoding and decoding, including an updated comment for the u property and modified regex for path serialization/deserialization. New test cases for orpc://localhost URLs have also been added.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Sep 29, 2025
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly fixes an issue with opaque URL paths by introducing orpc://localhost as a base URL, which improves compatibility with URL parsers. The changes are applied consistently across the codebase. I have a couple of suggestions to improve maintainability: one is to clarify a comment in the codec, and the other is to extract the hardcoded orpc://localhost string into a shared constant. Also, please note that a test case (on success - blob in packages/client/src/adapters/websocket/rpc-link.test.ts) appears to have been missed and still expects the old URL format.

Comment thread packages/standard-server-peer/src/codec.ts
Comment thread packages/standard-server-peer/src/codec.ts
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Sep 29, 2025

More templates

@orpc/arktype

npm i https://pkg.pr.new/@orpc/arktype@1046

@orpc/client

npm i https://pkg.pr.new/@orpc/client@1046

@orpc/contract

npm i https://pkg.pr.new/@orpc/contract@1046

@orpc/experimental-durable-iterator

npm i https://pkg.pr.new/@orpc/experimental-durable-iterator@1046

@orpc/hey-api

npm i https://pkg.pr.new/@orpc/hey-api@1046

@orpc/interop

npm i https://pkg.pr.new/@orpc/interop@1046

@orpc/json-schema

npm i https://pkg.pr.new/@orpc/json-schema@1046

@orpc/nest

npm i https://pkg.pr.new/@orpc/nest@1046

@orpc/openapi

npm i https://pkg.pr.new/@orpc/openapi@1046

@orpc/openapi-client

npm i https://pkg.pr.new/@orpc/openapi-client@1046

@orpc/otel

npm i https://pkg.pr.new/@orpc/otel@1046

@orpc/react

npm i https://pkg.pr.new/@orpc/react@1046

@orpc/react-query

npm i https://pkg.pr.new/@orpc/react-query@1046

@orpc/experimental-react-swr

npm i https://pkg.pr.new/@orpc/experimental-react-swr@1046

@orpc/server

npm i https://pkg.pr.new/@orpc/server@1046

@orpc/shared

npm i https://pkg.pr.new/@orpc/shared@1046

@orpc/solid-query

npm i https://pkg.pr.new/@orpc/solid-query@1046

@orpc/standard-server

npm i https://pkg.pr.new/@orpc/standard-server@1046

@orpc/standard-server-aws-lambda

npm i https://pkg.pr.new/@orpc/standard-server-aws-lambda@1046

@orpc/standard-server-fetch

npm i https://pkg.pr.new/@orpc/standard-server-fetch@1046

@orpc/standard-server-node

npm i https://pkg.pr.new/@orpc/standard-server-node@1046

@orpc/standard-server-peer

npm i https://pkg.pr.new/@orpc/standard-server-peer@1046

@orpc/svelte-query

npm i https://pkg.pr.new/@orpc/svelte-query@1046

@orpc/tanstack-query

npm i https://pkg.pr.new/@orpc/tanstack-query@1046

@orpc/trpc

npm i https://pkg.pr.new/@orpc/trpc@1046

@orpc/valibot

npm i https://pkg.pr.new/@orpc/valibot@1046

@orpc/vue-colada

npm i https://pkg.pr.new/@orpc/vue-colada@1046

@orpc/vue-query

npm i https://pkg.pr.new/@orpc/vue-query@1046

@orpc/zod

npm i https://pkg.pr.new/@orpc/zod@1046

commit: 17333c7

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 29, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@dinwwwh dinwwwh merged commit fc1437c into main Sep 29, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

URL pathname assignment fails for custom scheme (opaque path), causing RPC path to be lost

1 participant