fix(routes): track last route in routeNotFound #77835
Conversation
github-actions
bot
added
the
Scope: Frontend
|
|
||
| function useLastKnownRoute() { | ||
| const route = useRoutes(); | ||
| const lastRoute = usePrevious(route); |
There was a problem hiding this comment.
I wonder if we should consider excluding the route not found route, since it isn't really "known". Not that it would be a problem I think
There was a problem hiding this comment.
what do you mean by "exclude"?
michellewzhang
force-pushed
the
mz/track-last-route
branch
from
18ebbcc
to
b37b38f
Compare
| export default function LastKnownRouteContextProvider({children}: Props) { | ||
| const route = useRoutes(); | ||
| const prevRoute = usePrevious(route); | ||
| const lastKnownRoute = getRouteStringFromRoutes(prevRoute); |
There was a problem hiding this comment.
I wish we could just use the location state, but we call navigate in so many areas that it would be really annoying to update
| export const LastKnownRouteContext = createContext<string>(''); | ||
|
|
||
| export function useLastKnownRoute() { | ||
| return useContext(LastKnownRouteContext); |
There was a problem hiding this comment.
Do we care if this hook ever gets called outside of the provider?
There was a problem hiding this comment.
hmm, i guess it could possibly be useful elsewhere in the future, so it's not awful if it does get called outside of here. the only place we currently need it is inside routeNotFound.tsx
| @@ -27,13 +29,14 @@ function RouteNotFound({router, location}: Props) { | |||
| scope.setFingerprint(['RouteNotFound']); | |||
There was a problem hiding this comment.
You could alternatively set the fingerprint here, though it'd be less flexible
There was a problem hiding this comment.
i think i might prefer to do it in settings!
| children: React.ReactNode; | ||
| } | ||
|
|
||
| export const LastKnownRouteContext = createContext<string>('unknown'); |
There was a problem hiding this comment.
lol sorry, I was reviewing on my phone and it lost the <. I actually meant the string <unknown> so it's very easy to disambiguate it from a route named unknown or whatever
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
follow up to #77835 sometimes our last known route can be `/*`, which is the not found route. this isn't helpful for grouping, so we want to track routes distinct from that.
follow up to #77835 sometimes our last known route can be `/*`, which is the not found route. this isn't helpful for grouping, so we want to track routes distinct from that.
The problem
In Sentry today, tons of “route not found” events are grouped together into one issue, even if the suspect routes/transactions are not related. This poses a problem because the relevant teams are not alerted to these issues, which means
See an issue example here, and notice that not all the events have the same transaction.
The goal
Improve our grouping for “route not found” issues so that the events are split up into better issues — specifically, based on where the last known route was. That way, we can have more relevant issues grouped by (hopefully) the suspect problem route & triage more effectively.
Next steps
Next step would be setting the fingerprinting rules in the
javascriptproject to include the new tag-based grouping rule. This is in project settings:notion doc