blomqma / next-rest-framework

Type-safe, self-documenting APIs for Next.js
https://next-rest-framework.vercel.app
Other
134 stars 17 forks source link

ReadableStream passed to rpcOperation handler when using rpcRoute with App Router #132

Closed andydam closed 6 months ago

andydam commented 6 months ago

When using rpcRoute() with App Router, I noticed that calling the RPC Operation over the App Router route passes a ReadableStream instead of the actual values

Example:

// src/app/rpc/[operationId]/route.ts

import { rpcOperation, rpcRoute } from 'next-rest-framework';
import { z } from 'zod';

const schema = z.object({ testString: z.string() });

const testRpc = rpcOperation()
  .input(schema)
  .outputs([{ schema }])
  .handler(async (inputs) => {
    // inputs is actual a ReadableStream
    console.log(inputs);

    // this line will fail because testString property does not exist on ReadableStream
    return { testString: inputs.testString };
  });

export const { POST } = rpcRoute({ testRpc });

The RPC Operation works fine when called during SSR or when using rpcApiRoute() in Pages Router.

Looks like it has something to do with the request body's ReadableStream being passed directly to the handler (and middlewares)?

https://github.com/blomqma/next-rest-framework/blob/53999d54579d79672316a174ebe39f7256e70057/packages/next-rest-framework/src/app-router/rpc-route.ts#L113

It looks like rpcRoute() at least parses the body correctly for validation.

https://github.com/blomqma/next-rest-framework/blob/53999d54579d79672316a174ebe39f7256e70057/packages/next-rest-framework/src/app-router/rpc-route.ts#L83

There's a workaround where I can conditionally parse the ReadableStream manually in the handler but doesn't seem ideal, if that's the intent the argument types for the handler is wrong.

blomqma commented 6 months ago

Thanks for reporting, this is a bug that has now been fixed in v5.1.6, upgrading to that version should address your issue.

andydam commented 5 months ago

@blomqma Thanks for fixing that but I think there might be a bug with your fix, a

Next REST Framework encountered an error:
SyntaxError: Unexpected end of JSON input

error is returned if the rpcOperation is created without an input and if a body isn't passed in the request

Example:

// src/app/rpc/[operationId]/route.ts

import { rpcOperation, rpcRoute } from 'next-rest-framework';
import { z } from 'zod';

const testRpc = rpcOperation()
  .outputs([{ schema: z.string() }])
  .handler(() => {
    return 'asdf';
  });

export const { POST } = rpcRoute({ testRpc });
blomqma commented 5 months ago

@blomqma Thanks for fixing that but I think there might be a bug with your fix, a

Next REST Framework encountered an error:
SyntaxError: Unexpected end of JSON input

error is returned if the rpcOperation is created without an input and if a body isn't passed in the request

Example:

// src/app/rpc/[operationId]/route.ts

import { rpcOperation, rpcRoute } from 'next-rest-framework';
import { z } from 'zod';

const testRpc = rpcOperation()
  .outputs([{ schema: z.string() }])
  .handler(() => {
    return 'asdf';
  });

export const { POST } = rpcRoute({ testRpc });

@andydam You are correct, thanks for the follow-up report. The initial fix was too naive, causing this regression. I have now fixed it for good with better test coverage in v5.1.8, hope this resolves your issue.