If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Better telemetry for geo ip lookup failures

NEW
Unassigned

Status

()

Firefox
Search
P3
normal
Rank:
35
3 years ago
2 years ago

People

(Reporter: markh, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fxsearch])

(Reporter)

Description

3 years ago
The geoip lookups added in bug 1109120 should report telemetry for cases (other than timeouts) where we fail to get a country code - currently we only see that the request was not successful, but have no insight into why.

There are a couple of hurdles to this:

* We need to get the error reason from the XHR request.  This *seems* to be as simple as querying:

  request.channel.QueryInterface(Ci.nsIRequest).status

to get the NS_ERROR_* value in the onerror handler.

* It's not clear how "package" these codes for telemetry.  A reasonable first attempt might be to have a "keyed" histogram and to use the error code's module as the key (so, eg, we'd know if the problem is "network" or "security", but not be able to break it down further).  Or we might be able to use a key that captures both the specific codes we expect (presumably ones reflecting "no network" and "cert error due to captive portal") *and* simple module names for other errors.

* It's not clear how we would test this.  xpcshell tests do *not* get the list of hosts in server-locations.txt.  I can't find *any* xpcshell tests that use https:// (other than one spdy test that uses a node server, has special support in runxpcshelltests.py, and isn't enabled unless the user installs node and sets an env var to point at it)

And related to the above, I just noticed the test test_location_error.js does *not* do what I thought it did - it says:

  // from server-locations.txt, we choose a URL without a cert.
  let url = "https://nocert.example.com:443";

but in reality this test is just causing an NS_ERROR_UNKNOWN_HOST error rather than the a SEC_ERROR_*_CERTIFICATE error I was expecting.  However, I did manually check that a certificate error does cause onerror to be called (and the nsIRequest.status hack above to report the correct error) so this is a test-only issue - at a minimum the comments in the test should change, but ideally we'd work out how to get an actual certificate error.

Updated

3 years ago
Blocks: 1109120
Priority: -- → P3
Whiteboard: [fxsearch]

Updated

2 years ago
Rank: 35
You need to log in before you can comment on or make changes to this bug.