[server] If getting usage-based notifications fails, log a warning instead of propagating the error to the dashboard

[jankeromnes]

Description

If getting usage-based notifications fails, log a warning in server instead of propagating the error to the dashboard.

Related Issue(s)

Should fix #13131

by silencing this error which happens when you have multiple teams with usage-based billing enabled:

Request getNotifications unsuccessful: 455/"Multiple teams have billing enabled."

More context in Slack (internal):

Should we display the prompt when we see this error?

I'd rather catch it locally and ignore it for now.
The user will be prompted on next workspace start, anyway. 🧘

How to test

Hint: Reviewing without whitespace is easier: https://github.com/gitpod-io/gitpod/pull/13178/files?w=1

Release Notes

NONE

Documentation

Werft options:

• /werft with-preview
• /werft with-integration-tests=all
  Valid options are all, workspace, webapp, ide

[easyCZ]

Coming back to this, is this the correct approach? Just logging the error and returning nothing will only result in swalowing the error and not actually giving us a signal. Yes, we've got the log but that's not ultimately what we monitor so we'll not know (get alerted) when this is failing for users and they are not receiving their notifications.

What if we instead handled the notification subscription failing client side (not showing it) but we continued to throw the errors server side. That way, we can actually fix the underlying problem which seems errors to check the usage limit.

[jankeromnes]

Thanks @easyCZ! See also how we reached this consensus in Slack (internal).

Should we display the prompt when we see this error?

I'd rather catch it locally and ignore it for now.
The user will be prompted on next workspace start, anyway. 🧘

FYI, this (internal) was the identified underlying problem (that we're now logging as a warning instead of actually handling it):

Found it: When you call checkUsageLimitReached, it (correctly) wants to know the user's attributionId -- but that can be ambiguous (e.g. no explicit choice and two paying teams)

gitpod/components/server/ee/src/billing/billing-service.ts

Line 27 in c9aa527

const attributionId = await this.userService.getWorkspaceUsageAttributionId(user);

So, checkUsageLimitReached throws when attribution is ambiguous

Summary: We've decided not to handle the attribution ambiguity in getNotifications (because it's sort of a nice to have, and it's not the end of the world when it doesn't work), but instead wait until the user tries to start a workspace -- at which point the same ambiguity error happens, and causes a blocking modal to appear, encouraging users to lift the ambiguity by making an explicit attribution choice. After this point, the error is solved (so getNotifications will work as intended).

However, I do see your point: Maybe the catch-all should be more specific (i.e. log attribution ambiguity as a warning, but still re-throw any other error)?

[easyCZ]

However, I do see your point: Maybe the catch-all should be more specific (i.e. log attribution ambiguity as a warning, but still re-throw any other error)?

Indeed, that would make it more explicit (and also actually document the reason for this in code). As it stands, without that context I'd go and remove the try-catch-all