1844397 - Handle network errors, including OHTTP network config errors

Status: NEW

(Firefox :: Shopping, task, P3)

Description

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

In the patch for bug 1843067, I noted that the ShoppingProduct.request code currently calls await fetch without wrapping it in a try/catch. This means any network errors that cause fetch to throw will be uncaught, and be returned to the caller.

We also have a bunch of XPCOM services being connected together to make the OHTTP request calls work. I'm not sure if we get thrown Errors, but I hope so?

Regardless of the details, we'll want to be sure any/all network errors can be handled uniformly, and displayed to the user in some sort of "sorry, try again soon" catch-all UI.

Comment 1

[:Gijs (out for now; he/him)]

I'm not sure what errors would be returned by the OHTTP relayed requests - it looks like bug 1836241 covers this for DoH but there aren't clues there about what to look for...

Kershaw, do you know? Did this already get implemented for DoH-over-OHTTP?

Comment 2

[Kershaw Chang [:kershaw]]

I think Dana should know more details than me.
If I trace the code correctly, we get NS_ERROR_FAILURE if we don't have a valid config.

Comment 3

[Ted Campbell [:tcampbell]]

The key config can fail for all the normal network reasons and we use fetch so ideally nothing too special takes place.

Outside of the OHTTP envelope, the Relay can give us a few errors on behalf of itself or the gateway. This includes 500's for general failures, and 400/401 for decryption failures. We should ideally have a way to surface these so that we can do things like remove encryption key from caches.

Once we have the encryption to the gateway established, we have encapsulated errors that are similar to accessing any server/proxy with an nsChannel. If the Gateway disallows the specifc target the Relay will return 200 on the outer message, and an encrypted 403 in the inner message.

(For the iOS variant, I'm generally throwing specific exceptions for things like decryption failures. Things like timeouts or network connectivity already throw exceptions. If we successfully communicate with the Gateway, then a response object is returned to called with that status code which may still be a failure if the Target server gave an error or such)

Comment 4

[Dana Keeler (she/her) (use needinfo) [:keeler]]

An oblivious http channel should behave like an ordinary http channel (except for the extra stuff it does), so yes, I believe errors will be thrown in the same way.

Looking at the test (https://searchfox.org/mozilla-central/source/netwerk/test/unit/test_oblivious_http.js), it doesn't comprehensively cover those cases, though, so maybe it would be beneficial to extend those some.

Comment 5

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Since we have error handling in the product, the most we'd want to do here might be to add some additional telemetry around error cases.

I'm not sure this is worth blocking 119 on.