Cody: fix error notification display pattern for rate limit

[abeatrix]

RE https://sourcegraph.slack.com/archives/C052G9Y5Y8H/p1683290207798969?thread_ts=1683288519.959009&cid=C052G9Y5Y8H

Issue:

• A user who has hit the daily limit was getting a persistent error message as a system message that they can only get rid of if Cody is disabled.
• Send repeated notification is against VS code guideline

Proposed solution:

• This PR moves the error message to UI as a banner in chatview to address the pinpoint of getting a system popup repeatedly where the only solution is to disable Cody

• Issue: Users who are not on Chat view will not be notified that they have hit the quota when their sidebar is closed

• Solution: Display as error notification 4 time per day max (every 6 hours) and allow user to opt out from the notification

Edit: Updated for clarity.

Test plan

[sourcegraph-bot]

📖 Storybook live preview

[philipp-spiess]

How does moving the error message to the chat view resolve the issue you mentioned? Wouldn't it just move the problem to somewhere else? What happens if the sidebar is closed even?

There are two further improvements I had:

• Only show the rate limiting error once every 1hour or so, we can use a module scoped variable for this:

  let lastQuotaErrorShown: null | number = null;
  
  // later
  if (lastQuotaErrorShown === null || lastQuotaErrorShown < Date.now() - 60 * 60 * 1000) { 
   showError();
   lastQuotaErrorShown = Date.now()
  }

- The error message also contains a JSON error. We should be able to find out if it's a quota error from reading the HTTP headers and then wrap it so that it's a bit nicer to read.

[abeatrix]

How does moving the error message to the chat view resolve the issue you mentioned? Wouldn't it just move the problem to somewhere else?

I think the immediate problem was that the error message persisted and get fired repeatedly as a system message, which is against the VS Code Guideline (we got called out by MS before so wanted to be caution about using the notification 😅 )

What happens if the sidebar is closed even?

Yea that's a good point! What do you think about using https://code.visualstudio.com/api/ux-guidelines/status-bar

Only show the rate limiting error once every 1hour

Why does it need to be repeated? 🤔

Also, in the user's case, they were getting the daily quota pop-up while using the chat and not completion.

What do you think?

[philipp-spiess]

I think the immediate problem was that the error message persisted and get fired repeatedly as a system message, which is against the VS Code Guideline (we got caught by MS before so wanted to be caution about using the notification 😅 )

Yeah, 100% agree that this is a bad pattern. But if we show it in the sidebar, we also show it repeatedly, no?

Yea that's a good point! What do you think about using code.visualstudio.com/api/ux-guidelines/status-bar

Idk I’m not a big fan of the status bar, I always have it disabled. 😅

Why does it need to be repeated? 🤔

It resets after a while and then on the next day the quota can be hit again. Maybe we can even parse the timestamp in the message and hide it until that time.

Also, in the user's case, they were getting the daily quota pop-up while using the chat and not completion.

Hm but don't we render the error message in the chat view already? I feel like I have seen screenshots about this. It also looks from the code that your PR does not address this if the error pops up in the chat and not completion.

[abeatrix]

But if we show it in the sidebar, we also show it repeatedly, no?

Yes, but we have more control over it, and it won't be spamming their VS Code with notifications that would cause users to disable Cody as a result. because they can just keep the sidebar closed when Cody is down?

Hm but don't we render the error message in the chat view already?

When they send a message while being out of the quote, they will get an error message from Cody to remind them that their limit is hit, yes.

It also looks from the code that your PR does not address this if the error pops up in the chat and not completion.

Right! So that's the second issue --if you don't like the status bar and want to stay with the system pop-up, i think limiting it to once per day (about quote hit) will be less annoying than then getting reminded of something that's not working every hour. We should also add the Do not show again option as suggested by the VS Code guideline? What do you think? (If that sounds good i can update the PR real quick 👍 )

[philipp-spiess]

Yes, but we have more control over it, and it won't be spamming their VS Code with notifications that would cause users to disable Cody as a result. because they can just keep the sidebar closed when Cody is down?

Fair, this makes sense yeah.

Right! So that's the second issue --if you don't like the status bar and want to stay with the system pop-up, i think limiting it to once per day (about quote hit) will be less ignoring than then getting reminded of something that's not working every hour. We should also add the Do not show again option as suggested by the VS Code guideline? What do you think? (If that sounds good i can update the PR real quick 👍 )

Haha I’m not the definitive authority here, if you think the status bar would be better, I am all for it, just wanted to share my point of view :) Definitely agree on the once per day message though, 100% that we need this somehow.

"Don't show again" would also be really cool if that's possible?

[abeatrix]

if you think the status bar would be better, I am all for it,

I was just exploring options. I do not even know what's on my status bar so i'm with you on that 😆 I've been getting this error in my status bar that does take my attention though so that's what has inspired me hahaha

[philipp-spiess]

I was just exploring options. I do not even know what's on my status bar so i'm with you on that 😆 I've been getting this error in my status bar that does take my attention though so that's what has inspired me hahaha

Haha right? And there's so many widgets competing for attention too. Hiding it is bliss

[abeatrix]

@philipp-spiess i've updated the PR to address the second issue by limiting the error for rate limit to show ever 6 hours when true, do you think it's fair?

Video: https://www.loom.com/share/89301ada6b2d488cad24aa879201d0fc

[philipp-spiess]

Lets do this for now so we can ship a fix for this soonish (can you get to that today to also include your slash commands? otherwise we can do it on monday)

but long term I think we should do an overhaul of the error handling so that we don't have an rate limit error wrapped in a json syntax parsing error wrapped in a generic error and have to rely on the error message 😅

[abeatrix]

we should do an overhaul of the error handling

100% LOL Thanks for the quick review btw, did this with Cody completions and 🤯