Show detailed network errors in Felt login UI #337

gcp · 2026-01-13T16:31:21Z

When connecting to the enterprise console fails (e.g., due to an expired certificate), the error was a generic "NetworkError when attempting to fetch resource" which made debugging difficult.

This change:

Adds _xhrFetch() wrapper in ConsoleClient that uses XMLHttpRequest internally to access channel.status (a Gecko extension) on network errors
Converts status codes to readable names (e.g., SEC_ERROR_EXPIRED_CERTIFICATE) using nsINSSErrorsService
Displays the specific error in the Felt login UI
Replaces all fetch() calls with _xhrFetch() for consistent error handling (we could argue about this one)

lissyx

LGTM

gcp · 2026-01-14T07:36:15Z

Kind of want to add a test before landing.

gcp · 2026-01-14T14:56:53Z

Also let's wait until #291 lands so we can use the divs that adds to the Felt window.

gcp · 2026-01-14T19:12:05Z

browser/components/enterprise/modules/ConsoleClient.sys.mjs

+   * @param {string|null} [options.body=null] - Request body
+   * @returns {Promise<{ok: boolean, status: number, json: Function, text: Function}>}
+   */
+  _xhrFetch(url, { method = "GET", headers = {}, body = null } = {}) {


note: can't use async here because we already construct the Promise manually, it would get wrapped in another Promise. And we can't get rid of that promise because we need to resolve/reject in the callbacks from XMLHttpRequest.

From a callers POV it behaves the same as fetch(), minus the limitations in the comments.

1rneh

This is part 1 of my review. I had quite a lot of thoughts on our error report handling.
I’ll share a follow-up review (part 2) covering the changes to ConsoleClient later today.

1rneh · 2026-01-16T07:30:52Z

testing/enterprise/test_felt_browser_console_error.py

+        email = self.get_elem("#felt-form__email")
+        self._driver.execute_script(
+            """
+            arguments[0].value = arguments[1];
+            arguments[0].dispatchEvent(new Event('input', { bubbles: true }));
+            """,
+            email,
+            "random@mozilla.com",
+        )
+
+        btn = self.get_elem("#felt-form__sign-in-btn")
+        btn.click()


Could we add a method in FeltTests e.g. submit_email that we can call in this test (super.submit_email()) and in test_felt_00_chrome_on_email_submit in FeltTests to avoid duplicating code.

1rneh · 2026-01-16T07:31:23Z

testing/enterprise/test_felt_browser_console_error.py

+    def test_felt_0_load_sso(self, exp):
+        return True
+
+    def test_felt_1_perform_sso_auth(self, exp):
+        return True


The do nothing and can be removed, right?

1rneh · 2026-01-16T07:32:13Z

testing/enterprise/test_felt_browser_console_error.py

+            self._manually_closed_child = True
+        return super().teardown()
+
+    def test_felt_00_chrome_on_email_submit(self, exp):


Could we make the method name describe the test here?

1rneh · 2026-01-16T07:32:24Z

browser/extensions/felt/content/styles/felt.css

  font-size: medium;
-  /* stylelint-disable-next-line stylelint-plugin-mozilla/use-design-tokens */
-  color: var(--icon-color-critical);
+  color: var(--text-color-error);


1rneh · 2026-01-16T07:56:13Z

browser/extensions/felt/content/window.js

+function hideConnectionError() {
+  const errorContainer = document.querySelector(".felt-browser-error");
+  const errorElement = document.querySelector(".felt-browser-error-connection");
+  if (errorElement) {
+    errorElement.classList.add("is-hidden");
+  }
+  if (errorContainer) {
+    const crashError = errorContainer.querySelector(
+      ".felt-browser-error-multiple-crashes:not(.is-hidden)"
+    );
+    if (!crashError) {
+      errorContainer.classList.add("is-hidden");
+    }
+  }
+}


I guess this method generally resets the visible error state. Probably there will be more errors to be added to the list so let's maybe refactor the method to handel that.

Suggested change

function hideConnectionError() {

const errorContainer = document.querySelector(".felt-browser-error");

const errorElement = document.querySelector(".felt-browser-error-connection");

if (errorElement) {

errorElement.classList.add("is-hidden");

}

if (errorContainer) {

const crashError = errorContainer.querySelector(

".felt-browser-error-multiple-crashes:not(.is-hidden)"

);

if (!crashError) {

errorContainer.classList.add("is-hidden");

}

}

}

function resetErrorReport() {

const errorWrapper = document.querySelector(".felt-browser-error");

if (!errorWrapper.classList.contains("is-hidden")) {

// No visible error report.

return;

}

errorWrapper.classList.add("is-hidden");

const errors = errorWrapper.querySelectorAll(".felt-browser-error span:not(.is-hidden)");

errors.forEach(e => e.classList.add("is-hidden"));

}

On another thought, I think we could make the error reporting a bit more generic. I think we need the three methods:

ErrorReport.init() which we call once the document is loaded

ErrorReport.reset() when we retry the login flow

ErrorReport.update(error) if we encounter an error during the login process and want to add it to the error report.

And then we can have an enum will all errors we're facing to the user.

And then we can have an enum will all errors we're facing to the user.

Not sure about this part. The code is a bit messy because it tries to surface up the actual error instead of a generic "couldn't connect to the console", but this means enumeration would be problematic.

We could still display user-friendly error messages:

const ERR_MESSAGES = { CONSOLE_CONNECT: "We couldn't connect to the console.", MULTIPLE_CRASHES: "We couldn't launch Firefox Enterprise due to multiple crashes on startup." }

And if there is more to show such as the actual error message, we can append that to the error message or in a details section. What do you think?

1rneh · 2026-01-16T08:13:50Z

browser/extensions/felt/content/felt.xhtml

            data-l10n-id="felt-browser-error-multiple-crashes"
          >
          </span>
+          <span class="felt-browser-error-connection is-hidden"></span>


Could we convert them to div elements so they are not all displayed in one line?

1rneh · 2026-01-16T08:22:48Z

browser/extensions/felt/content/window.js

+  } catch (err) {
+    console.error(`FeltExtension: Failed to connect to console: ${err}`);
+    const consoleUrl = lazy.ConsoleClient.consoleBaseURI.origin;
+    showConnectionError(`Failed to connect to ${consoleUrl}: ${err.message}`);


We are localizing the crash error, so we should probably localize Failed to connect to as well.

Tbh I'm not so happy about what we display though. We should have something actionable for the user such as We couldn't connect to the console. Please contact your admin for support. and then a maybe a details section with that error message. Let's make sure we define the UX for this soon.

1rneh

Part 2:

Thank you for the detailed documentation :)

Just some small suggestion about the changes in ConsoleClient. The rest of it looks good.

1rneh · 2026-01-16T12:06:14Z

browser/components/enterprise/modules/ConsoleClient.sys.mjs

+      res = await this._xhrFetch(url, {
+        method: "POST",
+        headers: {
+          "Content-Type": "application/json",
+          Accept: "application/json",
+        },
+        body: JSON.stringify(devicePosture),
+      });


Hm, calling fetch directly should have been replaced here when we introduced _post in a766fb8. Seems like I missed this one. Could you change that? It also wraps _xhrFetch :)

Suggested change

res = await this._xhrFetch(url, {

method: "POST",

headers: {

"Content-Type": "application/json",

Accept: "application/json",

},

body: JSON.stringify(devicePosture),

});

res = await this._post(url, JSON.stringify(devicePosture));

And then in _fetch we also need to add headers.set("Accept", "application/json");.

1rneh · 2026-01-16T12:27:37Z

browser/components/enterprise/modules/ConsoleClient.sys.mjs

+        let errorName = "NetworkError";
+        if (xhr.channel) {
+          errorName = this._getErrorNameForStatus(xhr.channel.status);
+        }


nit :) (we already do it so nicely with the headers above)

Suggested change

let errorName = "NetworkError";

if (xhr.channel) {

errorName = this._getErrorNameForStatus(xhr.channel.status);

}

const errorName = xhr.channel

? this._getErrorNameForStatus(xhr.channel.status)

: "NetworkError"

1rneh · 2026-01-16T13:02:11Z

browser/components/enterprise/modules/ConsoleClient.sys.mjs

+        `ConsoleClient: Failed to connect to ${url}: ${err.message}`
+      );
+      throw err;
+    }


Logging and rethrowing the same error here seems redundant, as the caller already handles the logging. I’d suggest removing this catch unless we want to add semantic context to the error.

gcp requested a review from a team January 13, 2026 16:31

lissyx approved these changes Jan 14, 2026

View reviewed changes

gcp requested a review from 1rneh January 14, 2026 14:54

Show detailed network errors in Felt login UI

7f8fd1b

gcp force-pushed the felt-connection-error-visibility branch from c01676a to 7f8fd1b Compare January 14, 2026 19:09

gcp commented Jan 14, 2026

View reviewed changes

1rneh requested changes Jan 16, 2026

View reviewed changes

1rneh reviewed Jan 16, 2026

View reviewed changes

Show detailed network errors in Felt login UI #337

Are you sure you want to change the base?

Show detailed network errors in Felt login UI #337

Conversation

gcp commented Jan 13, 2026

Uh oh!

lissyx left a comment

Choose a reason for hiding this comment

Uh oh!

gcp commented Jan 14, 2026

Uh oh!

gcp commented Jan 14, 2026

Uh oh!

gcp Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

1rneh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

1rneh Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

1rneh Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

1rneh left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gcp Jan 14, 2026 •

edited

Loading

1rneh Jan 16, 2026 •

edited

Loading

1rneh Jan 16, 2026 •

edited

Loading

1rneh left a comment •

edited

Loading