Bug in APIs to resolve Issues

Dhrumil-Sentry commented 8 months ago

Environment

Steps to Reproduce

I received the following question from Linear:

We are marking Sentry issues as resolved when a linked Linear issue is closed. We do this using a PUT on the endpoint [https://sentry.io/api/0/issues/{issue_id}/](https://sentry.io/api/0/issues/%7Bissue_id%7D/) with: {"status":"resolvedInNextRelease"}

As far as I know this used to correctly mark it resolved in the NEXT release (similar to what you can do in the UI), but it appears to not work anymore. It just marks the issue as resolved in the current release. I also can't find anything about resolvedInNextRelease in your docs.

So I took a look at what the Sentry UI does and it appears it sends a PUT to [https://us.sentry.io/api/0/projects/{organization_slug}/{project_slug}/issues/?id={issue_id}](https://us.sentry.io/api/0/projects/%7Borganization_slug%7D/%7Bproject_slug%7D/issues/?id=%7Bissue_id%7D) with: {"status":"resolved","statusDetails":{"inNextRelease":true},"substatus":null}

This works. If I hit the same endpoint with that data it correctly shows up in the UI as marked as resolved in the next release.

But if I hit the endpoint we already use ([https://sentry.io/api/0/issues/{issue_id}/](https://sentry.io/api/0/issues/%7Bissue_id%7D/)) with: {"status":"resolved","statusDetails":{"inNextRelease":true},"substatus":null}

it ignores statusDetails and just marks it as resolved in the current release.

From the documentation it appears that bulk editing issues would maybe support it, but not when updating a single issue: https://docs.sentry.io/api/events/bulk-mutate-a-list-of-issues/ vs https://docs.sentry.io/api/events/update-an-issue/

As far as I can tell, that leaves me with two options:

Calling the endpoint that your UI does (which isn't documented) or the bulk mutate endpoint (to update a single issue) to get this functionality. Neither seems logical and both would require the project_slug too, which we don't currently store, but we could of course start doing that.
That you'll support statusDetails when updating a single issue. I don't see why it wouldn't be possible from a technical standpoint and having the API aligned seems right to me?

Expected Result

.

Actual Result

.

Product Area

Issues

Link

No response

DSN

No response

Version

No response

getsantry[bot] commented 8 months ago

Routing to @getsentry/product-owners-issues for triage ⏲️

scefali commented 8 months ago

@Dhrumil-Sentry I am unable to replicate the bug. Both endpoints use the same underlying logic to update the groups (this and this). So they should behave the same in this regard.

skbogner commented 8 months ago

Hi. I don't know what I'm missing. Here's a video of me trying to use the two apis and the different results I get. I can't speak for what state the issue actually ends up in, but at least the values I get back differ:

https://github.com/getsentry/sentry/assets/2521488/25e51690-092f-4b24-8301-96f970e7dd88

scefali commented 8 months ago

@skbogner Ok, I see the problem now. For some reason, the two endpoints have different serializer logic. One endpoint uses the output of update_groups directly and the other one uses the serializer. For whatever reason, the two versions have diverged in implementation. But the actual updating of data should be the same. You can verify this by making the GET request to the endpoint after each PUT request and comparing the content of those.

skbogner commented 8 months ago

They just don't behave similarly when I call them. I took two different unresolved issues that both returned the same beforehand:

    "status": "unresolved",
    "statusDetails": {},

One (1) I called https://sentry.io/api/0/issues/<issue_id>/ with:

{"status":"resolved","statusDetails":{"inNextRelease":true},"substatus":null}

The other (2) I called https://sentry.io/api/0/projects/linear/linear/issues/?id=<issue_id> with:

{"status":"resolved","statusDetails":{"inNextRelease":true},"substatus":null}

Then calling GET on each to compare I get different results:

For (1):
    "status": "resolved",
    "statusDetails": {
        "inRelease": "1.22152.0",

For (2):
    "status": "resolved",
    "statusDetails": {
        "inNextRelease": true,

There seems to be an additional feature/bug of once an issue has been resolved to either of these statuses subsequent unresolve/resolves always ends up in the same result. So if I unresolve (2) and then resolve it with https://sentry.io/api/0/issues/<issue_id>/ I end up with "inNextRelease": true. And if I unresolve (1) and call it with https://sentry.io/api/0/projects/linear/linear/issues/?id=<issue_id> I end up with "inRelease": "1.22152.0".

scefali commented 8 months ago

@skbogner I guess this is a real bug, adding it to our backlog

sandstrom commented 6 months ago

@scefali Just checking in, any update on this one?

This bug breaks the Sentry<>Linear integration.

Dhrumil-Sentry commented 6 months ago

We'll be working on fixing this soon and will comment here once we have updates

sandstrom commented 6 months ago

Thanks @Dhrumil-Sentry 😄

roggenkemper commented 6 months ago

@skbogner following up on this - i think there are 2 separate issues here that are happening.

1) With regards to your video, I think steve's initial guess was right in that the 2 serializers are behaving differently which is why the responses you get are different. 2) The second problem I believe is related to how "in next release" works. You mentioned trying out the two requests on two different issues and seeing different results in the GET responses. I think what might be causing the difference is that fact that the two issues might have been "last seen" in different releases. For example, if one of the issues was last seen a while ago while the other was seen very recently (in the most recent release), we will get different results because the "next release" for the first issue was already a release that exists, while the "next release" doesn't exist yet for the second issue.

We're working on fixing the first bug and adding a new feature that will allow you to "resolve in upcoming release" to solve the second problem stated

sandstrom commented 6 months ago

Just to chime in.

If I have this list of releases (using dates for simplicity):

2024-01-22
2024-03-13
2024-04-24

And I have an issue that hasn't been seen since January, if today is end of April, why would it ever make sense to mark it as "in next release", and have that mean 2024-03-13?

Because if that usage isn't common (which I'd argue it isn't), then I wouldn't add a new mode, I'd change (fix) the behavior of the current mode.

If you need to mark something as resolved in an already existing release, you'll know which one, and can just select it by name anyway. Or put differently, the reasonable use-case for this is when you want to mark something as resolved when merging to master (before a release exists), not doing retroactive marking of resolved several releases later.

roggenkemper commented 6 months ago

@sandstrom we're still relatively early on in the stages of trying to fix this use case, but we definitely appreciate the feedback. our main concern was breaking anyone's existing workflow if they do rely on the current implementation, but it is valid comment that it probably isn't super common

sandstrom commented 6 months ago

@roggenkemper Sounds good! Yeah, I think you'll save yourself some needless work by not introducing another mode.

skbogner commented 6 months ago

2. The second problem I believe is related to how "in next release" works. You mentioned trying out the two requests on two different issues and seeing different results in the GET responses. I think what might be causing the difference is that fact that the two issues might have been "last seen" in different releases. For example, if one of the issues was last seen a while ago while the other was seen very recently (in the most recent release), we will get different results because the "next release" for the first issue was already a release that exists, while the "next release" doesn't exist yet for the second issue.

Seconding what @sandstrom said. I would never expect "next release" to mean "the one after the one where the issue was last seen". That also feels to me like something you'd never want. That said I understand the very valid concern to not break any existing workflows that might rely on this. Let us know what you end up with 👍

getsentry / sentry