Skip to content
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

Next.js 13+ app directory support #3

Open
chrisjh opened this issue Jun 1, 2023 · 9 comments
Open

Next.js 13+ app directory support #3

chrisjh opened this issue Jun 1, 2023 · 9 comments

Comments

@chrisjh
Copy link

chrisjh commented Jun 1, 2023

Are there any plans to add Next.js 13 app directory support to this package?

There's a few things that don't quite work the same between pages and app directory related to structured logging and API routes:

The new app route handler in Next.js 13 has specific named exports (GET, POST, PUT, DELETE). The current version of logtail-nextjs wraps the pages api handler with withLogtail(handler) which no longer works with the new named exports. Additionally, Next changed the types to NextResponse and NextRequest from NextApiResponse and NextApiRequest.

@curusarn
Copy link
Contributor

Hi @chrisjh,

Thank you for reaching out!

Next.js 13+ directory structure is definitely something we want to support.
We are tracking this issue internally. I can't share an ETA at this point.
I will keep you updated here once I have anything more to share.

Thanks again for raising this. I appreciate it. 🙏

@nles
Copy link

nles commented Aug 17, 2023

Is there a simple way to get this to work with Next 13? Would be great!

@marceloclp
Copy link

I put together a quick implementation for this. I am not sure yet how to differentiate between an API handler and a Route handler, but the code below should work.

type NextRouteHandler = (
  request: NextRequest,
  context: NextRouteHandlerContext
) => Promise<NextResponse> | NextResponse;
type NextRouteHandlerContext = { params: Record<string, string | string[]> };

export type LogtailNextRequest = NextRequest & { log: Logger }
export type LogtailRouteHandler = (
  request: LogtailNextRequest,
  context: NextRouteHandlerContext
) => Promise<NextResponse> | NextResponse;

export function withLogtailRouteHandler(handler: LogtailRouteHandler): NextRouteHandler {
  return async (req, ctx) => {
    const report: RequestReport = {
      startTime: new Date().getTime(),
      path: `${req.nextUrl.pathname}${req.nextUrl.search}`,
      method: req.method,
      host: req.headers.get('host') || '',
      userAgent: req.headers.get('user-agent') || '',
      scheme: 'https',
      ip: req.ip,
      region: config.region,
    }
    const runtime = (
      typeof globalThis.EdgeRuntime === 'undefined' &&
      process.env.NEXT_RUNTIME != 'edge'
    ) ? 'lambda' : 'edge';
    const logger = new Logger({}, report, false, runtime);
    const logtailRequest = req as LogtailNextRequest;
    logtailRequest.log = logger;

    try {
      const res = await handler(logtailRequest, ctx);
      logger.attachResponseStatus(res.status);
      await logger.flush();
      return res;
    } catch (error: any) {
      logger.error('Error in Route Handler', { error });
      logger.attachResponseStatus(500);
      await logger.flush();
      throw error;
    }
  }
}

AFAIK, route handlers need to return a NextResponse or Response object. That means we don't need to hijack the response object methods so that we can ensure to flush all logs before the response is sent out. We can simply wait for the handler to resolve, flush the logs, and finally return the response.

Happy to open a PR if this is acceptable :)

@chrisjh
Copy link
Author

chrisjh commented Sep 21, 2023

Thanks @marceloclp - nice to see others are interested in a solution here! Can you share an example of how this works with Next.js 13's app directory named route handlers (e.g. GET, POST, PATCH, etc.). As far as I can tell, you can't wrap these named exports like you used to be able to in the pages router.

Next expects the route.ts file to look something like this:

# /app/api/route.ts

export async function GET(request: Request) {}

https://nextjs.org/docs/app/building-your-application/routing/route-handlers#convention

@marceloclp
Copy link

marceloclp commented Sep 21, 2023

It should be fine to use arrow functions instead:

export const GET = withLogtailRouteHandler((req) => {
  return NextResponse.json({})
})

export const PUT = async () => {}

An alternative could be:

export withLogtailRouteHandler(
  function GET() {}
) {}

export withLogtailRouteHandler(
  function PUT() {}
) {}

But AFAIK it virtually doesn't matter if you use arrow functions as Next does not make use of this

@chrisjh
Copy link
Author

chrisjh commented Sep 21, 2023

Ah, clever! Makes sense, will try it out in our app today.

@adasq
Copy link

adasq commented Nov 26, 2023

Thank you for the interim solution. Looking forward to having it out of the box, cheers!

@logemann
Copy link

@curusarn after 1,5 years, it would be nice to finally see the implementation in the lib. This should be a quick thing to do right?

@logemann
Copy link

logemann commented Nov 14, 2024

I put together a quick implementation for this. I am not sure yet how to differentiate between an API handler and a Route handler, but the code below should work.

export function withLogtailRouteHandler(handler: LogtailRouteHandler): NextRouteHandler {
  return async (req, ctx) => {
    const report: RequestReport = {

nice code. But trying to use it in my test route gives me a build error.

/../.next/types/app/api/test2/route.ts:166:7 - error TS2344: Type '{ __tag__: "POST"; __param_position__: "second"; __param_type__: NextRouteHandlerContext; }' does not satisfy the constraint 'ParamCheck<RouteContext>'.
  The types of '__param_type__.params' are incompatible between these types.
    Type 'Record<string, string | string[]>' is missing the following properties from type 'Promise<any>': then, catch, finally, [Symbol.toStringTag]

166       {
          ~
167         __tag__: 'POST'
    ~~~~~~~~~~~~~~~~~~~~~~~
... 
169         __param_type__: SecondArg<MaybeField<TEntry, 'POST'>>
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
170       },
    ~~~~~~~


Found 1 error in ../../../.next/types/app/api/test2/route.ts:166

Cant get a handle on this. Most likely a Promise thing? Hard to read....

UPDATE:

this fixed it:

type NextRouteHandlerContext = {
    params: Promise<Record<string, string | string[]>>;
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants