Bug: Auto-summarizing does not work

[bdougie]

Describe the bug

@open-sauced/triage can you confirm if this feature is broken for you.

related #2657

Steps to reproduce

1. Go to https://highligts.new
2. Add a github pr link
3. Click the auto-summarize button.
4. Confirm if it summarized.

Browsers

No response

Additional context (Is this in dev or production?)

Code of Conduct

• I agree to follow this project's Code of Conduct

Contributing Docs

• I agree to follow this project's Contribution Docs

[github-actions]

Thanks for the issue, our team will look into it as soon as possible! If you would like to work on this issue, please wait for us to decide if it's ready. The issue will be ready to work on once we remove the "needs triage" label.

To claim an issue that does not have the "needs triage" label, please leave a comment that says ".take". If you have any questions, please reach out to us on Discord or follow up on the issue itself.

For full info on how to contribute, please check out our contributors guide.

[CBID2]

This is something I have noticed too @bdougie

[bdougie]

Yeah seems like it's been broken a while. I think the placement of the feature doesn't encourage a lot of feedback.

We should prioritize this bug soon @brandonroberts bit it is not high priority right now.

[takanome-dev]

Worked for me 🤔

Screen.Recording.2024-03-18.at.18.18.46.mov

[bdougie]

Worked for me 🤔

Interesting, can you confirm it works when you select an item from the suggestions list as well @takanome-dev?

[nickytonline]

Can you both share your network panel outputs? I'm pretty sure we have an issue for this already where it gives a 401 @jpmcb?

[FatumaA]

works for PRs, errors for issues

the error as below:

[nickytonline]

So there's the 401 error and as a result, no comments array.

[takanome-dev]

Disconnected and reconnected + clear cache

👀 @bdougie @nickytonline

Screen.Recording.2024-03-18.at.19.13.16.mov

[nickytonline]

If it happens again, we can catch the 401 potentially and ask the user to logout and log back in, but we need to mention why. Any idea why someone might have to log back in @brandonroberts? We busted the cache recently in a release, so not sure what else it might be.

[bdougie]

Disconnected and reconnected + clear cache

I can confirm that this is fixed for me when clearing the cache.

[jpmcb]

Here's the old issue where we identified this was a 401 issue: #1440 (and associated issues were we identified something similar)

But looks like it was closed as complete in January.

Works ok for me:

[bdougie]

Here's the old issue where we identified this was a 401 issue: #1440 (and associated issues were we identified something similar)

But looks like it was closed as complete in January.

Interesting. I think passing an error message via the API would clear these types of errors quicker in the future. It seems like there are a few ways this feature fails, but we are guessing every time.

1. Credits are unavailable (OpenSauced needs to pay)
2. No comments in issues
3. Auth token (this is fixed).

[nickytonline]

For credits unavailable, we could give a friendly message that the service is unavailable and log to Sentry the actual error to notify us to add credits, and for no comments we can use the Elvis operator, i.e. nullish coalescing so the array map doesn't throw.

Also, can we get notifications about low credits?

[brandonroberts]

If it happens again, we can catch the 401 potentially and ask the user to logout and log back in, but we need to mention why. Any idea why someone might have to log back in @brandonroberts? We busted the cache recently in a release, so not sure what else it might be.

Not sure why you would need to logout and login for this as its using authentication through our API. The other issues we've had are because the GitHub session doesn't get refreshed without logging out/back in.

[jpmcb]

Interesting. I think passing an error message via the API would clear these types of errors quicker in the future.

...

For credits unavailable, we could give a friendly message that the service is unavailable and log to Sentry the actual error to notify us to add credits

I actually disagree: the API for many backend/back-office operations (like OpenAI credits running out) should not be surfaced to clients. Otherwise, we reveal bits and pieces of the state of our infrastructure, generally increasing the attack surface area of the entire stack by threat actors.

This, in my opinion, is actually a observability and logging problem: we don't currently consume logs from the API in a way that can trigger alerts to our engineering team. While we do have this setup on Sentry, as Nick pointed out, this would be the wrong place to surface a back-office problem: instead of errors of this nature being surfaced to clients, we would want to invest in logging infrastructure and alerting mechanisms in order for our internal team to become aware of these issues (i.e., Grafana Loki consumes logs and Grafana Alerts triggers a configured alert based on a FATAL type log).

[bdougie]

we would want to invest in logging infrastructure and alerting mechanisms in order for our internal team to become aware of these issues (i.e., Grafana Loki consumes logs and Grafana Alerts triggers a configured alert based on a FATAL type log).

@jpmcb I am not sure if there is a log tracking issue or if this should go into the log repo. But could we link it there and then close this. I think adding more observability to this is the move, but not a priority for now.

[jpmcb]

@bdougie yes, I have https://github.com/open-sauced/api/issues/693 which will be a good lift: our current deployment using Digital Ocean Apps doesn't have great support for log sinks that aren't DataDog or PaperTail.

We can continue to track this in that issue.