fix(grouping): Fix buggy chained exception filtering

[lobsterkatie]

This fixes two bugs with the way we handle chained exceptions when grouping. In the first case, we assume that there will always be a root exception (an exception with exception_id = 0). In the second, we assume that an exception will never be listed as its own parent. Both of those are reasonable assumptions to make about a well-formed tree, but for some reason, sometimes the data we get doesn't work that way, and we end up hitting an error.

Though we handle that error gracefully, it would be better if we didn't error out at all. To fix this, this PR checks for those cases explicitly and handles them directly, before they can cause an error.

The effects of this change can be seen in two places in the tests: We no longer have to exclude certain test cases from our check that no exception has been logged, and one test case which previously was having one of its exceptions wrongly excluded from grouping now includes the full chain.

Fixes #73592
Fixes https://sentry.my.sentry.io/organizations/sentry/issues/2293485
Fixes https://sentry.sentry.io/issues/6634847839
Fixes https://sentry.my.sentry.io/organizations/sentry/issues/2293421

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #93966      +/-   ##
==========================================
- Coverage   88.04%   88.03%   -0.01%     
==========================================
  Files       10356    10356              
  Lines      598264   598266       +2     
  Branches    23232    23232              
==========================================
- Hits       526720   526709      -11     
- Misses      71076    71089      +13     
  Partials      468      468              

[yuvmen]

Looks good, a question and a minor suggestion you can freely ignore

[yuvmen]

minor - The analytical part of my brain is thinking that the ids_seen thing is basically detecting circles, so why are we treating circles of 2 different than circles of more than that, so we probably have a way to do this without explicitly checking the mechanism.parent_id, and simply detecting that we already saw this ID.
But there is something to be said about clarity, and for putting out there in a more forward manner that this is something that is possible and we detect for, so if you feel like keeping it like this thats totally fine too.

[lobsterkatie]

It's a valid point, but I like the clarity, and also, just relying on ids_seen wouldn't catch the case where there's only one exception, but it's listed as its own parent.

I suppose we could replace this check with checking if the parent is in ids_seen as well, but then we still have the same number of checks and IMHO it'd be less clear what problem we're trying to solve.

[yuvmen]

just asking, you have more context here - in terms of testing I am just wondering how isolated is the impact of this change, is it just how we group, or are we possibly letting through errors with a structure we didnt let through before, and we need to make sure our system can handle such errors? Its totally possible these errors always went through we just grouped them wrong and so theres less to worry about

[lobsterkatie]

Yeah, the only thing this whole parent/child setup affects is the hash calculation and how we lay the errors out on the page in issue details. So this will indeed change the hash value for events like this, but it'll change them to what they should have been all along. (And this isn't a super common case, so the impact in terms of noise should hopefully be pretty minimal.)