BlairCurrey / trpc-koa-adapter

Add trpc to your koa server
https://www.npmjs.com/package/trpc-koa-adapter
MIT License
22 stars 5 forks source link

tRPC mutations hang with Koa adapter (solved; due to bodyParser middleware conflict) #24

Closed aseemk closed 8 months ago

aseemk commented 9 months ago

Hi there. Thanks for this library! I hit an issue using it, where queries worked, but mutations would hang indefinitely, and never actually execute my procedure/function.

I managed to figure this issue out, but I thought I'd document it here for anyone else that runs into it (since I spent more than an hour debugging it).

The cause was that:

Fortunately:

So the solution is: if you're using @koa/bodyparser, set patchNode: true for tRPC to work with Koa!

  app.use(
    bodyParser({
      // This is required for tRPC to work! Since this `bodyParser` middleware reads & drains the
      // request stream, while tRPC tries to stream the body too by default, which then hangs forever.
      // The only way tRPC short-circuits that if a `body` is already populated on the native Node
      // request object, which this `patchNode` option does.
      patchNode: true,
      // `encoding` needs to be explicitly specified for the TypeScript type (likely wrong).
      encoding: "utf-8",
    })
  );

One last note: we were actually using koa-bodyparser — a subtly different package name that stopped getting updates after v4 — while the patchNode option was only added in v5! So fixing this also required changing & upgrading from koa-bodyparser to @koa/bodyparser.

Consider adding a note about this to the readme. Even though this isn't directly related to this Koa tRPC adapter, it might not be uncommon for Koa users to be using the bodyparser middleware too.

Thanks and hope this helps others!

BlairCurrey commented 8 months ago

Hi @aseemk. Thank you for this detailed writeup, very helpful. I see the issue as you describe. I am doing a little more research but will definitely incorporate your feedback here to make it easier for others.

Those requiring the old koa-bodyparser may be able to utilize use its disableBodyParser option. Probably in conjunction with createKoaMiddleware's prefix. Something like this is working with mutations for me:

const prefix = "/trpc";

const app = new Koa();

app.use(async (ctx, next) => {
  if (ctx.path.startsWith(prefix)) ctx.disableBodyParser = true;
  await next();
});

app.use(bodyParser());

app.use(
  createKoaMiddleware({
    router: appRouter,
    prefix,
  })
);

FWIW this should work with @koa/bodyparser as well.

If it happens that you can add the bodyparser middleware after this createKoaMiddleware then that would avoid this problem as well.

Also wondering if there is anything more we can do in this library to handle this (or fail more gracefully) such as a looking at the raw body, but not sure if that ultimately makes sense.

EDIT: Actually, I think the disableBodyParser workaround is effectively equivalent to adding createKoaMiddleware before the body parser middleware because the body will not be parsed and available in koa prior to reaching the tRPC router.

BlairCurrey commented 8 months ago

I made a fix that solves it across the board (confirmed with koa-bodyparser and @koa/bodyparser at least). patchNode with @koa/bodyparser will still work but wont be necessary.

I added a check for the parsed body on ctx.request and add it to the node request if found. I imagine that covers the vast majority of scenarios but I also added a note in the readme briefly stating the expection for parsed bodies to found on the ctx.request just in case.

EDIT: This definitely seems like the way to go. I played around with the trpc express adapter and tried to produce the same error. Parsing the body with express.json() (body-parser under the hood) before adding the adapter middleware does not produce this issue because body-parser puts the body on the req (which we are now doing here as well).