Skip to content

Conversation

@ChristopherChudzicki
Copy link
Contributor

@ChristopherChudzicki ChristopherChudzicki commented Oct 16, 2024

What are the relevant tickets?

Closes https://github.com/mitodl/hq/issues/5771

How can this be tested?

Trigger some errors in the frontend and check that (a) appropriate UI is shown, (b) errors show up in Sentry

  1. Now, set env vars... You something specific to you for the sentry environment name
    # in env/frontend.local.env
    NEXT_PUBLIC_SENTRY_DSN="https://8131564a9b9e31ac3cbcf7952f0fea80@o48788.ingest.us.sentry.io/4506260054540288"
    # suggest using your initials
    NEXT_PUBLIC_SENTRY_ENV=abc-test
  2. Visit https://mit-office-of-digital-learning.sentry.io/issues/?end=2024-10-16T16%3A09%3A48&environment=cc-test&project=4506260054540288&start=2024-10-16T16%3A08%3A47 and change the env filter to your environment
  3. Restart the nextjs app (e.g., restart docker) and trigger some errors.
  4. Async Errors: Visit /sentry-example-page; click Throw async error!
    • The errors should show up in your sentry environment
  5. Sync Errors: Visit /sentry-example-page; click Render error!
    • synchronous errors during rendering crash the app unless they are caught by an error boundary. We catch the error with error.ts. You should see a generic error page.
    • The error should show up in your sentry environment
  6. Forbidden Errors: Visit /sentry-example-page; click Forbidden error!
    • You should get the Not Allowed page
    • The error should show up in your sentry environment
  7. Root Layout Error:
    • Production only: This uses global-error.ts, which only functions when running in production mode. So to test this, you need to run NextJS locally in production mode. This should be easier than it is, but for now:
    # If not using docker, skip to `NODE_ENV...` commands below.
    docker compose stop watch
    docker compose run --rm --service-ports watch bash
    
    # now within the watch container:
    NODE_ENV=production yarn workspace main build
    NODE_ENV=production yarn workspace main next start
    
    • (if you're not using docker, run the last two NODE_ENV... commands in your terminal)
    • Now that you're running in production mode, click "Root Layout Error (production only)
      " in the header. You should see an error page, but no header/footer. (The error occurred in the header. We can't show it.)

Before merging

  • Remove the testing page and header button
  • merge ol-infra PR

@ChristopherChudzicki ChristopherChudzicki changed the base branch from main to nextjs October 16, 2024 18:38
return config
},

productionBrowserSourceMaps: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During production builds, they are disabled to prevent you leaking your source on the client, unless you specifically opt-in with the configuration flag.

From docs

Seems like silly obfuscation to me.

Anyway, our source is public. I think keeping the source maps public will mean we don't need to upload them to sentry, but we'll only know for sure after we deploy it.


const { withSentryConfig } = require("@sentry/nextjs")

module.exports = withSentryConfig(nextConfig, {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This, and a lot of stuff below, was injected automatically by @sentry/wizard CLI

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be removed prior to merging. (Added by @sentry/wizard and modified a bit, but useful for testing this PR.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file was unnecessary... NextJS has the error boundaries builtin, we just provide relevant UI at error.ts pages.

I removed it because it was also a little buggy... It had this render method:

  render() {
    if (this.state.hasError && isForbiddenError(this.state.error)) {
      return <ForbiddenPage />
    }
    return this.props.children
  }

which meant that the error was silently suppressed if it was not a ForbiddenError. (The error "should" have been re-thrown, but really we should have just used error.tsx.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I extremely doubt we will ever use this file, but @sentry/wizard added it, and there's no real harm in having it.

@ChristopherChudzicki ChristopherChudzicki marked this pull request as ready for review October 16, 2024 20:20
@ChristopherChudzicki ChristopherChudzicki changed the title Cc/sentry NextJS Sentry Integration Oct 16, 2024
@gumaerc gumaerc self-assigned this Oct 17, 2024
@ChristopherChudzicki ChristopherChudzicki added the Needs Review An open Pull Request that is ready for review label Oct 17, 2024
Copy link
Contributor

@gumaerc gumaerc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM

Just remember to remove that example page and header error button

@ChristopherChudzicki ChristopherChudzicki merged commit 2d308c5 into nextjs Oct 17, 2024
12 checks passed
ChristopherChudzicki added a commit that referenced this pull request Oct 18, 2024
* configure sentry, fix error pages

* add env vars to CI

* add global styles to global-error

* TEMPORARY self destruct button

* cleanup

* cleanup error boundary, other misc cleanup

* remove a few useless things

* add a simple test i guess

* remove intentional error triggers
jonkafton added a commit that referenced this pull request Oct 21, 2024
* Merge `main` into `nextjs` (#1687)

* Add location field to LearningResource and LearningResourceRun models (#1604)

* Always delete past events during ETL, and filter them out from api results too just in case (#1660)

* Release 0.21.1

* Release date for 0.21.1

* remove unnecessary padding adjustment on Input base styles (#1666)

* remove unnecessary padding adjustment on Input base styles

* adjust actual textarea padding to match Figma

* Content File Score Adjustment (#1667)

* Release 0.21.2

* Restore program letter intercept view (#1643)

* Restore program letter intercept view

* Generate schema

* Fix test

* fixing import

* Revert "fixing import"

This reverts commit cf56807.

---------

Co-authored-by: shankar ambady <ambady@mit.edu>

* Release date for 0.21.2

* topic detail banner / subtopic logic revisions (#1646)

* add new darker chip variant

* move subtopic display to the banner

* remove max width on banner subtitle texts

* query subtopics only on parent topic channels, and on child topic channels query "related topics" aka the topic's siblings

* don't display the current topic in related topics

* show parent topic breadcrumbs on child topic channel pages

* fix tests

* only do subtopics mocking if necessary

* fix editchannelpage test

* move action buttons under the title on mobile

* fix test

* add new default channel background image

* "toopics"

* fix background position

* remove accidentally committed file

* fix background size on mobile

* remove unnecessary order css rule

* fix banner background on mobile

* fix breadcrumb props

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* remove unnecessary topic id filter

* remove unnecessary breadcrumb fallback

* set the default banner background image in the banner component itself, and only apply large and up 140% background size if the default image is being used

* properly set default background image

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Update Yarn to v4.5.0 (#1624)

* Update Yarn to v4.5.0

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Clear Cache on Deploy (#1668)

* adding initial cache clear command

* adding cache clear command to heroku release script

* fix docstrings

* removing invalid flag from clear cache command (#1675)

* prevent featured course carousel from re-randomizing (#1673)

* use skipFeatured when invalidating userlists / learning paths after update

* add a new function for updating query data for user_list_parents and learning_path_parents on the featured courses list when either is edited without re-randomizing

* fix learning path invalidations

* fix test

* Shuffling around where the search_update event is fired so it happens in more places (#1679)

- Changed HeroSearch to use the SearchField component instead of SearchInput, so we get the event capture stuff
- Updated SearchInput to not require setPage (for use on the homepage)
- Moved facet events out of the SearchPage and into SearchDisplay so they get called on other pages that use it
- Also trigger the facet events on more things - should also now get sorting changes, choosing between categories, and clearing facets

* Reinstate background steps image

---------

Co-authored-by: Matt Bertrand <mrbertrand@gmail.com>
Co-authored-by: Doof <mitx-devops@mit.edu>
Co-authored-by: Carey P Gumaer <gumaerc@mit.edu>
Co-authored-by: Anastasia Beglova <abeglova@mit.edu>
Co-authored-by: Nathan Levesque <rhysyngsun@users.noreply.github.com>
Co-authored-by: shankar ambady <ambady@mit.edu>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Shankar Ambady <shanbady@gmail.com>
Co-authored-by: James Kachel <james@jkachel.com>

* restoring up and down chevrons (#1690)

* Config to set cache control headers (#1700)

* Middleware to set cache headers

* Set headers in the config

* Upgrade Next.js

* Regex to exclude paths with a file extension to apply only to routes

* Remove middleware file

* Instruct to only store in shared cache (not browsers)

* Copy yarn releases to the Docker container (fixes build) (#1703)

* Copy yarn releases

* Build the docker image on all push

* NextJS - re-enable program letter tests (#1696)

* fixing program letter template

* re-enabling test

* adding space

* remove unnecessary webpack customizations (#1704)

* Increase cache duration (#1705)

* Increase cache durations

* Update comment

* Prod deployment. Add Posthog vars

* NextJS Sentry Integration (#1701)

* configure sentry, fix error pages

* add env vars to CI

* add global styles to global-error

* TEMPORARY self destruct button

* cleanup

* cleanup error boundary, other misc cleanup

* remove a few useless things

* add a simple test i guess

* remove intentional error triggers

* Appzi env vars and Sentry config

* Move deploy jobs to respective workflow. Docker build for dry run

---------

Co-authored-by: Matt Bertrand <mrbertrand@gmail.com>
Co-authored-by: Doof <mitx-devops@mit.edu>
Co-authored-by: Carey P Gumaer <gumaerc@mit.edu>
Co-authored-by: Anastasia Beglova <abeglova@mit.edu>
Co-authored-by: Nathan Levesque <rhysyngsun@users.noreply.github.com>
Co-authored-by: shankar ambady <ambady@mit.edu>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Shankar Ambady <shanbady@gmail.com>
Co-authored-by: James Kachel <james@jkachel.com>
Co-authored-by: Chris Chudzicki <christopher.chudzicki@gmail.com>
@odlbot odlbot mentioned this pull request Oct 22, 2024
74 tasks
@rhysyngsun rhysyngsun deleted the cc/sentry branch February 7, 2025 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review An open Pull Request that is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants