-
-
Notifications
You must be signed in to change notification settings - Fork 840
feat(request): add cloneRawRequest utility for request cloning #4382
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #4382 +/- ##
==========================================
- Coverage 91.32% 91.31% -0.01%
==========================================
Files 173 173
Lines 11084 11115 +31
Branches 3201 3204 +3
==========================================
+ Hits 10122 10150 +28
- Misses 961 964 +3
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi @kamaal111 Why I didn't implement cloning the request is that it will introduce significant performance degradation. The app: import { Hono } from '../../src'
const app = new Hono()
app.post('/', async (c) => {
const data = await c.req.json()
return c.json(data)
})
export default appBenchmark command: Result:
We should find a good way to resolve the problem without performance delegation. |
|
I understand, thank you @yusukebe! Something like this: export class HonoRequest {
// ...
new = () => {
return new Request(this.url, {
// Collect request init from this class without accessing `raw`
})
}
}That would then get consumed as the following: const app = new Hono()
app.post(
'/login',
zValidator(
'json',
z.object({
username: z.string(),
})
),
(c) => {
const validated = c.req.valid('json')
// `auth` from better-auth for example
const response = await auth.handler(c.req.new());
}
)Would it be worth it to contribute this to this project? |
9184511 to
ced5737
Compare
I'm also thinking of familiar things. Please wait until my vacation ends. |
|
I'm working on this issue, but I can't find a good solution. Can you share more details of your idea? |
|
@yusukebe I have workaround for it in my own project that only requires a Request object with JSON and it looks something like this: import type { HonoContext } from '../api/contexts.js';
export async function makeNewRequest(c: HonoContext, options?: { withBody?: boolean }): Promise<Request> {
let body: string | undefined = undefined;
if (options?.withBody) {
const rawBody: unknown = await c.req.json();
body = JSON.stringify(rawBody);
}
const requestInit: RequestInit = { method: c.req.method, headers: c.req.header(), body };
return new Request(c.req.url, requestInit);
}But for Hono I am thinking something like this: // ...
// Type safe transformer for request init
const BODY_TYPE_TO_TRANSFORMER_MAPPING: { [K in keyof Body]: (body: unknown) => Body[K] } = {
json: (body) => JSON.stringify(body),
text: (body) => body as string,
arrayBuffer: (body) => body as ArrayBuffer,
blob: (body) => body as Blob,
formData: (body) => body as FormData,
}
export class HonoRequest {
// ...
new = async (): Promise<Request> => {
const requestInit: RequestInit = { method: this.method, headers: this.header() }
const cachedKeys = Object.keys(this.bodyCache) as Array<keyof typeof this.bodyCache>
// If no body is cached but the raw request has a body and it's not consumed, consume it
if (cachedKeys.length === 0 && this.raw.body && !this.raw.bodyUsed) {
const body = await this.raw.text()
this.bodyCache.text = body
requestInit.body = body
return new Request(this.url, requestInit)
}
const firstKey = cachedKeys[0] as keyof Body | undefined
// If no cached body, return request without body
if (firstKey == null) return new Request(this.url, requestInit)
const cachedBody = await this.bodyCache[firstKey]
requestInit.body = BODY_TYPE_TO_TRANSFORMER_MAPPING[firstKey](cachedBody) ?? cachedBody
return new Request(this.url, requestInit)
}
}I haven't tested this in my application yet though, just drafted it quickly, but I will write proper tests for this and also see how it behaves in my app. |
|
Just confirming that the following works fine for my project now. const app = new Hono()
app.post(
'/login',
zValidator(
'json',
z.object({
username: z.string(),
})
),
(c) => {
const validated = c.req.valid('json');
// New Request object constructor method, that is valid even after consumption.
const request = await c.req.new();
// `auth` from better-auth for example
const response = await auth.handler(request);
}
) |
|
I have pushed my updated solution, let me know if its alright |
|
Hi @kamaal111 Adding a |
|
I understand @yusukebe Excuse me if I am making mistakes writing the following, because I can only write this from my phone for the coming 2 weeks. class HonoRequest {
// ...
// internal raw Request object which was previously `raw`
private _raw: Request
get raw() {
// check if raw has been consumed
// if it has been consumed then return a new Request object with cached values
// else return raw in this class
}
}Note that we don't need a promise with the getter, because we have 2 simple cases and we don't need to think about forcing a new RequestObject if the raw object has not been consumed. This should fix the issue of using raw while it is not consumed, but to not cause regressions in performance we likely need to reference the non getter raw prober and the getter raw property would be used for public applications. What do you think about this solution? |
|
I think adding a raw getter is good, but how to create a new Request object if it has been consumed? The problem is that cached values are get raw() {
if (this._raw.bodyUsed) {
// get body from the cached values?
// but the values in this.bodyCache are Promise
return new Request(this._raw, {
...this._raw,
body, // how to get this?
})
}
return this._raw
} |
|
@yusukebe you're right I missed that fact. We will then also have access the body cache directly without the convenience method. Then we can make a |
|
Hey @kamaal111 Nice suggestion. The performance degradation is not big with the solution. I've created the new PR: #4425 What do you think of it? |
|
Hey @yusukebe |
|
Thanks! I'll check it. |
|
Sorry to interrupt. What is
|
|
Thank you @usualoma for your comment! The main problem I have been facing is that I would like to run validating middleware before I enter my handler, and my handler body would invoke better-auth's handler, as described here: https://hono.dev/examples/better-auth I know my use case is quite unique, but my goal is to have the endpoints documented with OpenAPI. So for this cloning it outside of HonoRequest would not work, because the body has already been consumed before I get access to the raw Request object in my handler. I think either the solution that @yusukebe presented in his PR would work or the solution in this PR (mine) if we want to keep changing it in Hono. Alternatively we can also make better-auth work with a "consumed" Request object, but this is a very short term solution as there might be other third party integrations in the future that would like to use the Request object somehow. |
|
Interrupting is no problem! Thank you for the idea. Making a helper is nice to keep the HonoRequest clean. I may make it without the problem @kamaal111 mentioned. I'll work on it later. |
|
Hi @usualoma Sorry! Your @kamaal111 For example, the following works without errors. app.post(
'/',
validator('json', (data) => {
return data
}),
async (c) => {
const rawRequest = await cloneRawRequest(c.req)
auth.handler(rawRequest)
// ...
}
)Is this what you want to have? I think it's a good idea to make a utility function rather than adding logic to |
|
Hi @kamaal111 We can include import { cloneRawRequest } from 'hono/request-helper'However, I'm unsure if @usualoma Do you have any idea for the naming? |
|
Well, generally speaking, it seems natural that both the “class” and “class-related utility methods” are exported from "hono/request". import { HonoRequest, cloneRawRequest } from 'hono/request'However, I recognize that Hono's existing helpers aren't structured that way. So, as you say, maybe I feel like putting it in |
017281a to
00620b2
Compare
|
@yusukebe I have updated this PR to export the |
f0df438 to
a330010
Compare
**Problem** After using Hono validators, the `raw` Request obejct gets consumed during body parsing, making it unusable for external libraries like `better-auth`. This results in the error: ```shell TypeError: Cannot construct a Request with a Request object that has already been used. ``` **Root Cause** The issue occurs in the `#cachedBody` method in `HonoRequest`. When parsing request bodies (json, text, etc.), the method directly calls parsing methods on the raw Request object, which consumes its body stream. Once consumed, the Request cannot be cloned or reused by external libraries. **Solution** Adding a utility function to clone HonoRequest's underlying raw Request object, handling both consumed and unconsumed request bodies. Signed-off-by: Kamaal Farah <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
|
Hey @kamaal111 Let's go with this! I'll merge this into the |
|
Awesome thank you for the support 🙏 |

Problem
After using Hono validators, the
rawRequest obejct gets consumedduring body parsing, making it unusable for external libraries like
better-auth.This results in the error:
Root Cause
The issue occurs in the
#cachedBodymethod inHonoRequest. When parsing requestbodies (json, text, etc.), the method directly calls parsing methods on the raw Request
object, which consumes its body stream. Once consumed, the Request cannot be cloned or
reused by external libraries.
Solution
Adding a utility function to clone HonoRequest's underlying raw Request
object, handling both consumed and unconsumed request bodies.
The author should do the following, if applicable
bun run format:fix && bun run lint:fixto format the code