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

[Bug?]: Sentry is outdated and setup causes type check errors #11714

Open
1 task done
phil714 opened this issue Nov 4, 2024 · 2 comments
Open
1 task done

[Bug?]: Sentry is outdated and setup causes type check errors #11714

phil714 opened this issue Nov 4, 2024 · 2 comments
Labels
bug/needs-info More information is needed for reproduction

Comments

@phil714
Copy link

phil714 commented Nov 4, 2024

What's not working?

I tried adding the using the command yarn redwood setup monitoring sentry and noticed that my type checking job was failing because of there was a typing mismatch between Sentry.ErrorBoundary fallback props and the FatalErrorPage (specifically, the DevFatalErrorPage) . Tried reproducing on a new repository and got the same problem.

I got around that by ignoring the typing there but doesn't seems like the best solution.

I also noticed that the package is very outdated, the way the browser tracing integration works is deprecated now

Sentry.init({
  dsn,
  environment,
  integrations: [new Sentry.BrowserTracing()],
  tracesSampleRate: 1.0,
})
'BrowserTracing' is deprecated.ts(6385)
browsertracing.d.ts(124, 4): The declaration was marked as deprecated here.
⚠ Error(TS6385)  | 
BrowserTracing is deprecated.
(alias) new BrowserTracing(_options?: Partial<BrowserTracingOptions>): Sentry.BrowserTracing
export BrowserTracing
The Browser Tracing integration automatically instruments browser pageload/navigation actions as transactions, and captures requests, metrics and errors as spans.

The integration can be configured with a variety of options, and can be extended to use any routing library. This integration uses {@see IdleTransaction} to create transactions.

@deprecated — Use browserTracingIntegration() instead.

How do we reproduce the bug?

  • yarn create redwood-app my-redwood-project --typescript
  • yarn redwood setup monitoring sentry
  • yarn rw type-check
✔ Generating the Prisma client...
✔ Generating types
src/App.tsx:16:34 - error TS2322: Type '(props: { error?: ErrorWithRequestMeta; }) => Element' is not assignable to type 'ReactElement<any, string | JSXElementConstructor<any>> | FallbackRender'.
  Type '(props: { error?: ErrorWithRequestMeta; }) => Element' is not assignable to type 'FallbackRender'.
    Types of parameters 'props' and 'errorData' are incompatible.
      Type '{ error: Error; componentStack: string; eventId: string; resetError(): void; }' is not assignable to type '{ error?: ErrorWithRequestMeta; }'.
        Types of property 'error' are incompatible.
          Type 'Error' is not assignable to type 'ErrorWithRequestMeta'.
            Property 'graphQLErrors' is missing in type 'Error' but required in type '{ mostRecentRequest?: RequestDetails; graphQLErrors: EnhancedGqlError[]; mostRecentResponse?: any; }'.

16   <Sentry.ErrorBoundary fallback={FatalErrorPage}>
                                    ~~~~~~~~~~~~~~~~

  ../node_modules/@redwoodjs/web/dist/components/DevFatalErrorPage.d.ts:14:5
    14     graphQLErrors: EnhancedGqlError[];
           ~~~~~~~~~~~~~
    'graphQLErrors' is declared here.
  src/App.tsx:16:34
    16   <Sentry.ErrorBoundary fallback={FatalErrorPage}>
                                        ~~~~~~~~~~~~~~~~
    Did you mean to call this expression?

What's your environment? (If it applies)

System:
    OS: macOS 14.2.1
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.17.0 - /private/var/folders/st/3_r4c9h55pq9kzv_7h6bk0dr0000gn/T/xfs-4a8c6746/node
    Yarn: 4.5.1 - /private/var/folders/st/3_r4c9h55pq9kzv_7h6bk0dr0000gn/T/xfs-4a8c6746/yarn
  Databases:
    SQLite: 3.43.2 - /usr/bin/sqlite3
  Browsers:
    Chrome: 117.0.5938.88
    Safari: 17.2.1
  npmPackages:
    @redwoodjs/core: 8.4.0 => 8.4.0
    @redwoodjs/project-config: 8.4.0 => 8.4.0
  redwood.toml:
    [web]
      title = "Redwood App"
      port = 8910
      apiUrl = "/.redwood/functions" # You can customize graphql and dbauth urls individually too: see https://redwoodjs.com/docs/app-configuration-redwood-toml#api-paths
      includeEnvironmentVariables = [
        # Add any ENV vars that should be available to the web side to this array
        # See https://redwoodjs.com/docs/environment-variables#web
      ]
    [api]
      port = 8911
    [browser]
      open = true
    [notifications]
      versionUpdates = ["latest"]

Are you interested in working on this?

  • I'm interested in working on this
@phil714 phil714 added the bug/needs-info More information is needed for reproduction label Nov 4, 2024
@Tobbe
Copy link
Member

Tobbe commented Nov 5, 2024

Thanks for reporting @phil714. Would you be interesting in trying to upgrade our Sentry integration? A PR would be more than welcome 🙏

@phil714
Copy link
Author

phil714 commented Nov 6, 2024

Thanks for reporting @phil714. Would you be interesting in trying to upgrade our Sentry integration? A PR would be more than welcome 🙏

Would gladly upgrade Sentry but I'm not exactly sure what is the best way to solve the typing issue. The FatalErrorPage typing doesn't match with what can be sent to the Sentry.ErrorBoundary given that it is both used for dev and production build and the Sentry.ErrorBoundary seems to be geared toward a production error fallback.

I feel like we should we should keep the original FatalErrorBoundary that is there before we configure sentry and just pass the non-dev error to Sentry but it seems like it wouldn't work with the way the FatalErrorPage is created right now. Something like that:

  // ./src/App.tsx
  <FatalErrorBoundary page={FatalErrorPage}>
    <Sentry.ErrorBoundary fallback={MyErrorPage}>
      <RedwoodProvider titleTemplate="%PageTitle | %AppTitle">
        <RedwoodApolloProvider>{children}</RedwoodApolloProvider>
      </RedwoodProvider>
    </Sentry.ErrorBoundary>
  </FatalErrorBoundary>
// ./src/pages/FatalErrorPage.tsx
import { DevFatalErrorPage } from '@redwoodjs/web/dist/components/DevFatalErrorPage'

export default DevFatalErrorPage || MyErrorPage;

For this to work, the default RedwoodJS project should split the FatalErrorPage into two components which I'm not sure is desirable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/needs-info More information is needed for reproduction
Projects
None yet
Development

No branches or pull requests

2 participants