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

Typing issue with the custom _error page in TypeScript #5448

Closed
3 tasks done
simontaisne opened this issue Jul 22, 2022 · 4 comments · Fixed by #5463
Closed
3 tasks done

Typing issue with the custom _error page in TypeScript #5448

simontaisne opened this issue Jul 22, 2022 · 4 comments · Fixed by #5463
Labels
Package: nextjs Issues related to the Sentry Nextjs SDK Type: Bug

Comments

@simontaisne
Copy link

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which package are you using?

@sentry/nextjs

SDK Version

7.7.0

Framework Version

12.2.2

Link to Sentry event

No response

Steps to Reproduce

  1. Run the wizard command
npx @sentry/wizard -i nextjs
  1. Rename the generated_error.js custom page to _error.tsx
  2. Provide the ErrorProps type from next/error to the CustomErrorComponent
import NextErrorComponent from 'next/error';
import type { NextPage } from 'next';
import type { ErrorProps } from 'next/error';
import * as Sentry from '@sentry/nextjs';

const CustomErrorComponent: NextPage<ErrorProps> = ({ statusCode }) => {
  return <NextErrorComponent statusCode={statusCode} />;
};

CustomErrorComponent.getInitialProps = async (context) => {
  await Sentry.captureUnderscoreErrorException(context);
  // -------------------------------------------^^^^^

  return NextErrorComponent.getInitialProps(context);
};

export default CustomErrorComponent;

Expected Result

No typing issue with TypeScript.

Actual Result

Argument of type 'NextPageContext' is not assignable to parameter of type 'ContextOrProps'.
  Index signature for type 'string' is missing in type 'NextPageContext'.
@simontaisne
Copy link
Author

It looks like the typing issue can be fixed this way.

CustomErrorComponent.getInitialProps = async (context) => {
  const errorInitialProps = await NextErrorComponent.getInitialProps(context);

  await Sentry.captureUnderscoreErrorException(errorInitialProps);

  return NextErrorComponent.getInitialProps(context);
};

Not sure if this is expected though?

@AbhiPrasad
Copy link
Member

@lobsterkatie could you take a look?

@AbhiPrasad AbhiPrasad added Status: Needs Reproduction Package: nextjs Issues related to the Sentry Nextjs SDK labels Jul 25, 2022
@lobsterkatie
Copy link
Member

lobsterkatie commented Jul 25, 2022

It looks like the typing issue can be fixed this way.

CustomErrorComponent.getInitialProps = async (context) => {
  const errorInitialProps = await NextErrorComponent.getInitialProps(context);

  await Sentry.captureUnderscoreErrorException(errorInitialProps);

  return NextErrorComponent.getInitialProps(context);
};

Not sure if this is expected though?

The types check, but I would expect you wouldn't get the data you want on your sentry error in that case.

What's happening here is we're running into microsoft/TypeScript#15300. The two solutions I can find are:

  1. Remove the index signature from our type.
  2. Call our function on a copy of the context object: await Sentry.captureUnderscoreErrorException({ ...contextData }).

All things being equal, 1) is better, because it doesn't require any change on our users' part. I was reluctant to do it, because I expected it to break a case in which someone who still has the legacy captureUnderscoreErrorException in their component function wants to pass custom props from getInitialProps. In other words, I expected it to break the TS version of this (which is why I included the index signature in the first place):

const CustomErrorComponent = (props) => {
  // people might still have this
  Sentry.captureUnderscoreErrorException(props); // I would expect this to error in TS

  console.log(props.stuff);

  return <NextErrorComponent statusCode={props.statusCode} />;
};

CustomErrorComponent.getInitialProps = async (contextData) => {
  // In case this is running in a serverless function, await this in order to give Sentry
  // time to send the error before the lambda exits
  await Sentry.captureUnderscoreErrorException(contextData);

  // This will contain the status code of the response
  const result = NextErrorComponent.getInitialProps(contextData);
  // IRL this would probably come from a 'fetch' call
  result.stuff = "things";
  return result;
};

As it turns out, though, in order to do that you have to jump through enough hoops that our function doesn't seem to mind:

type GetInitialPropsResult = ErrorProps & { stuff?: string };

const CustomErrorComponent: NextPage<GetInitialPropsResult> = (
  props: GetInitialPropsResult
) => {
  // people might still have this
  Sentry.captureUnderscoreErrorException(props); // I would expect this to error in TS

  console.log(props.stuff);

  return <NextErrorComponent statusCode={props.statusCode} />;
};

CustomErrorComponent.getInitialProps = async (
  contextData: NextPageContext
): Promise<GetInitialPropsResult> => {
  // In case this is running in a serverless function, await this in order to give Sentry
  // time to send the error before the lambda exits
  await Sentry.captureUnderscoreErrorException(contextData);

  // This will contain the status code of the response
  const result = NextErrorComponent.getInitialProps(
    contextData
  ) as GetInitialPropsResult;
  // IRL this would probably come from a 'fetch' call
  result.stuff = "things";
  return result;
};

I will admit I'm still confused why it doesn't mind, but it seems not to mind - our SDK builds, my test app's _error page builds... So I'll pull the index signature.

lobsterkatie added a commit that referenced this issue Jul 26, 2022
…n` argument type (#5463)

This removes the index signature in the `ContextOrProps` type in our nextjs `_error` helper function, `captureUnderscoreErrorException`, as it's causing build errors for people writing their `_error` page in TS.

Note: The problem I anticipated[1], which led me to add it in the first place, hasn't materialized in my testing. 

Fixes #5448.

[1] #5448 (comment)
@Geczy
Copy link

Geczy commented Jan 25, 2023

ran into this error today @lobsterkatie can you reopen?
image

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: nextjs Issues related to the Sentry Nextjs SDK Type: Bug
Projects
None yet
4 participants