sergiodxa / remix-auth

Simple Authentication for Remix
https://sergiodxa.github.io/remix-auth/
MIT License
2k stars 110 forks source link

Request clone method does not clone the body in bun #285

Closed Kuechlin closed 2 weeks ago

Kuechlin commented 4 months ago

Describe the bug

The request body is empty in Strategy.authenticate() when using bun as runtime.

In src/authenticator.ts a new Request is created but the body is not cloned in bun.

  new Request(request.url, request),

It is a know bug in bun issue, but is not getting fixed soon.

A option to disable cloning the request would fix the issue for me.

Your Example Website or App

none

Steps to Reproduce the Bug or Issue

use bun

Expected behavior

A option to disable cloning the request

Screenshots or Videos

No response

Platform

Additional context

to fix the issue for me i have to override the authenticate method without cloning the request

function authenticate(
  this: any,
  strategy: string,
  request: Request,
  options: Pick<
    AuthenticateOptions,
    "successRedirect" | "failureRedirect" | "throwOnError" | "context"
  > = {}
): Promise<User> {
  const strategyObj = this.strategies.get(strategy);
  if (!strategyObj) throw new Error(`Strategy ${strategy} not found.`);
  return strategyObj.authenticate(request, this.sessionStorage, {
    throwOnError: this.throwOnError,
    ...options,
    name: strategy,
    sessionKey: this.sessionKey,
    sessionErrorKey: this.sessionErrorKey,
    sessionStrategyKey: this.sessionStrategyKey,
  });
}

// @ts-expect-error
authenticator.authenticate = authenticate.bind(authenticator);
kentcdodds commented 2 weeks ago

This is causing me an issue in Node as well. After this line runs, request.headers changes from a Headers object to an Array<[string, string]>. Any idea why this might be?

kentcdodds commented 2 weeks ago

Any reason to not use request.clone()?

kentcdodds commented 2 weeks ago

Nevermind. My issue was completely unrelated and fixed by MSW: https://github.com/mswjs/interceptors/commit/2f6106c9a96447c08423cef3ae68fde1c9e28b65

bitofbreeze commented 2 weeks ago

This was just fixed in Bun v1.1.27 Does that fix your issue @Kuechlin ?

I'm wondering if something similar in workerd could be causing my issues https://github.com/cloudflare/workers-sdk/issues/3259

Kuechlin commented 2 weeks ago

Thank you for the info, Bun v1.1.27 fixes the issue.