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

fix(website): enable error page for 500s #12651

Closed
wants to merge 6 commits into from
Closed

Conversation

zchsh
Copy link
Contributor

@zchsh zchsh commented Jan 14, 2022

👀 500 Error page Preview (vs live)
👀 404 Error page Preview (vs live)

[ WIP ]


@zchsh
Copy link
Contributor Author

zchsh commented Jan 19, 2022

@brkalow tagging ya here cause I've started abstracting out components/error-view, as a candidate to be moved into react-components.

As I was doing this, ended up pulling out a useErrorPageAnalytics hook, which feels a bit more re-usable than the error-view components (which is essentially just that hook bundled with some layout-specific UI).

Seeing at least a few paths forward here, wanted to get your take on this before diving in further / opening PRs on other repos:

  1. Move components/error-view into react-components, publish as @hashicorp/react-error-view, don't expose useErrorPageAnalytics hooks, since we don't need it (yet).
  2. Do the above, but expose useErrorPageAnalytics as a named export, idea being that it seems likely want to re-use the error-view analytics logic, but not the layout-specific styling, on other projects, namely dev-portal.
  3. Move useErrorPageAnalytics into @hashicorp/platform-analytics instead of bundled int he react-component. Would still make @hashicorp/react-error-view, would have an underlying dep on platform-analytics. Idea here being platform-analytics seems intended as the home for all analytics stuff. But maybe this should apply to top-level analytics stuff only (eg initializing window.analytics), and not specific calls (useErrorPageAnalytics just calls window.analytics, expects it to be set-up beforehand)?
  4. Something else?

Very open to input here! 🙇‍♂️

@brkalow
Copy link
Contributor

brkalow commented Jan 19, 2022

@zchsh My first thought was to place it in the analytics package, but if that feels like too many layers of abstraction I think it'd be fine to place it in react-error-view for now and just not export it 👍

@dstaley
Copy link
Contributor

dstaley commented Jan 19, 2022

Definitely a fan of providing the useErrorPageAnalytics hook somehow. No strong feelings whether it's a named export, from a package in react-components, or whether it's provided by platform-analytics. I'm leaning towards agreeing with Bryce that it makes the most sense with the other analytics stuff, but I can also see dependencies being hairy to manage when we're trying to coordinate a breaking change to the API.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants