fix(compass-global-writes): handle loading error COMPASS-8446

[paula-stacho]

Description

This one adds a LOADING_ERROR state. This state is reached whether the loading fails in the initial state (NOT_READY), or during polling (SHARDING). I have considered to simply show the toast during polling and continue with polling attempts. But seeing it in action, I think the toast isn't very clear when there was no user action, so I choose to handle both the same way (LOADING_ERROR and user has to reload). Let me know what you think about it, I'm on the edge about stopping the polling after it fails - it might be a temporary error after all.

Checklist

• New tests and/or benchmarks are included
• Documentation is changed or added
• I have signed the MongoDB Contributor License Agreement (https://www.mongodb.com/legal/contributor-agreement)

Motivation and Context

• Bugfix
• New feature
• Dependency update
• Misc

Open Questions

Dependents

Types of changes

• Backport Needed
• Patch (non-breaking change which fixes an issue)
• Minor (non-breaking change which adds functionality)
• Major (fix or feature that would cause existing functionality to change)

[gribnoysup]

I have considered to simply show the toast during polling and continue with polling attempts

I think another option to consider here is that during polling we might just continue polling without notifications, logging errors should be enough for us to trace down the issue if users report something like "polling never ends". But also I think what you went with is also a good approach, constantly notifying with toasts also doesn't make much sense to me.

No strong preference though (generally probably would make sense to just stick with whatever mms is doing in these cases, or the closes approximation of it) so approved 👍

[gribnoysup]

This special test api being a catch-all for all the tests in this file is getting a bit hard to read from my perspective, especially because it seems like most of these methods are added for just one test case. If you need a one-off mocking / stubbing for a special test case like that I would really recommend doing it right in your test:

it('fails when ...', function() {
  sinon.stub(atlasService, 'authenticatedFetch').rejects(...)
  // ... more of the test here
})

That way you're not binding your test to a shared mock implementation which usually makes tests easier to maintain in the long run and easier to understand what's being mocked when reading through the test case

[paula-stacho]

I get your point, but in this case the tests were getting very verbose and difficult to read because 'authenticatedFetch' handles 4 different things. So often the tests were repeating the whole auth fetch mock logic section only to change 1 thing. The only one that would be easy to mock and only used once would be failsToFetchClusterDetails - because that is the very first request that happens in the flow, and it's only on init. That one I did just for consistency.