redwoodjs / redwood

The App Framework for Startups
https://redwoodjs.com
MIT License
17.35k stars 1k forks source link

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

Open phil714 opened 4 weeks ago

phil714 commented 4 weeks ago

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?

16


  ../node_modules/@redwoodjs/web/dist/components/DevFatalErrorPage.d.ts:14:5
    14     graphQLErrors: EnhancedGqlError[];
'graphQLErrors' is declared here.

src/App.tsx:16:34 16


    Did you mean to call this expression?
   ```

### What's your environment? (If it applies)

```shell
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?

- [X] I'm interested in working on this
Tobbe commented 3 weeks ago

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

phil714 commented 3 weeks ago

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.