Bug 1635460 - Show 4XX and 5XX Network requests as errors in the console. r=Honza.
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Tue, 02 Jun 2020 13:31:24 +0000
changeset 597595 189c39e5f2d18f87252b670f58bfef39bb765b59
parent 597594 b38d112a2d6ae41c6039edfca99983216656bc3b
child 597596 9af9690fcef1f8f9377dc6fc876eccf205c51389
push id13310
push userffxbld-merge
push dateMon, 29 Jun 2020 14:50:06 +0000
treeherdermozilla-beta@15a59a0afa5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersHonza
bugs1635460
milestone79.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1635460 - Show 4XX and 5XX Network requests as errors in the console. r=Honza. Differential Revision: https://phabricator.services.mozilla.com/D77465
devtools/client/themes/webconsole.css
devtools/client/webconsole/components/Output/message-types/NetworkEventMessage.js
devtools/client/webconsole/reducers/messages.js
devtools/client/webconsole/test/browser/browser_webconsole_filters.js
devtools/client/webconsole/test/browser/sjs_slow-response-test-server.sjs
devtools/client/webconsole/test/node/store/filters.test.js
devtools/client/webconsole/test/node/store/search.test.js
devtools/client/webconsole/utils/messages.js
--- a/devtools/client/themes/webconsole.css
+++ b/devtools/client/themes/webconsole.css
@@ -727,16 +727,22 @@ a.learn-more-link.webconsole-learn-more-
   color: var(--theme-highlight-blue);
   font-style: inherit;
 }
 
 .webconsole-app .message.network .network-monitor .empty-notice {
   font-size: 16px;
 }
 
+/* For 4XX and 5XX requests, display the text in the "error" color */
+.webconsole-app .message.network.error .message-flex-body .message-body .url,
+.webconsole-app .message.network.error .message-flex-body .message-body .status {
+  color: currentColor;
+}
+
 .network.message .network-info {
   display: none;
   margin-block: 6px 2px;
   border: solid 1px var(--theme-splitter-color);
 }
 
 .network.message.open .network-info {
   display: block;
--- a/devtools/client/webconsole/components/Output/message-types/NetworkEventMessage.js
+++ b/devtools/client/webconsole/components/Output/message-types/NetworkEventMessage.js
@@ -10,17 +10,20 @@ const {
   createElement,
 } = require("devtools/client/shared/vendor/react");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const Message = createFactory(
   require("devtools/client/webconsole/components/Output/Message")
 );
 const actions = require("devtools/client/webconsole/actions/index");
-const { l10n } = require("devtools/client/webconsole/utils/messages");
+const {
+  isMessageNetworkError,
+  l10n,
+} = require("devtools/client/webconsole/utils/messages");
 
 loader.lazyRequireGetter(
   this,
   "TabboxPanel",
   "devtools/client/netmonitor/src/components/TabboxPanel"
 );
 const {
   getHTTPStatusCodeURL,
@@ -75,16 +78,20 @@ function NetworkEventMessage({
     timeStamp,
   } = message;
 
   const { response = {}, totalTime } = networkMessageUpdate;
 
   const { httpVersion, status, statusText } = response;
 
   const topLevelClasses = ["cm-s-mozilla"];
+  if (isMessageNetworkError(message)) {
+    topLevelClasses.push("error");
+  }
+
   let statusCode, statusInfo;
 
   if (
     httpVersion &&
     status &&
     statusText !== undefined &&
     totalTime !== undefined
   ) {
--- a/devtools/client/webconsole/reducers/messages.js
+++ b/devtools/client/webconsole/reducers/messages.js
@@ -1,15 +1,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
 const {
   isGroupType,
+  isMessageNetworkError,
   l10n,
 } = require("devtools/client/webconsole/utils/messages");
 
 const constants = require("devtools/client/webconsole/constants");
 const { DEFAULT_FILTERS, FILTERS, MESSAGE_TYPE, MESSAGE_SOURCE } = constants;
 
 loader.lazyRequireGetter(
   this,
@@ -552,24 +553,37 @@ function messages(
         ...state,
         messagesPayloadById: new Map(messagesPayloadById).set(
           action.id,
           action.data
         ),
       };
 
     case constants.NETWORK_MESSAGE_UPDATE:
-      return {
+      const updatedState = {
         ...state,
         networkMessagesUpdateById: {
           ...networkMessagesUpdateById,
           [action.message.id]: action.message,
         },
       };
 
+      // If the request status code is a 4XX or 5XX, then we may have to display the
+      // message (as an error).
+      if (isMessageNetworkError(action.message)) {
+        return setVisibleMessages({
+          messagesState: updatedState,
+          filtersState,
+          prefsState,
+          uiState,
+        });
+      }
+
+      return updatedState;
+
     case UPDATE_REQUEST:
     case constants.NETWORK_UPDATE_REQUEST: {
       const request = networkMessagesUpdateById[action.id];
       if (!request) {
         return state;
       }
 
       return {
@@ -1195,17 +1209,18 @@ function isGroupClosed(groupId, messages
  */
 function passNetworkFilter(message, filters) {
   // The message passes the filter if it is not a network message,
   // or if it is an xhr one,
   // or if the network filter is on.
   return (
     message.source !== MESSAGE_SOURCE.NETWORK ||
     message.isXHR === true ||
-    filters[FILTERS.NET] === true
+    filters[FILTERS.NET] === true ||
+    (filters[FILTERS.ERROR] && isMessageNetworkError(message))
   );
 }
 
 /**
  * Returns true if the message shouldn't be hidden because of the xhr filter state.
  *
  * @param {Object} message - The message to check the filter against.
  * @param {FilterState} filters - redux "filters" state.
@@ -1213,34 +1228,36 @@ function passNetworkFilter(message, filt
  */
 function passXhrFilter(message, filters) {
   // The message passes the filter if it is not a network message,
   // or if it is a non-xhr one,
   // or if the xhr filter is on.
   return (
     message.source !== MESSAGE_SOURCE.NETWORK ||
     message.isXHR === false ||
-    filters[FILTERS.NETXHR] === true
+    filters[FILTERS.NETXHR] === true ||
+    (filters[FILTERS.ERROR] && isMessageNetworkError(message))
   );
 }
 
 /**
  * Returns true if the message shouldn't be hidden because of levels filter state.
  *
  * @param {Object} message - The message to check the filter against.
  * @param {FilterState} filters - redux "filters" state.
  * @returns {Boolean}
  */
 function passLevelFilters(message, filters) {
   // The message passes the filter if it is not a console call,
   // or if its level matches the state of the corresponding filter.
   return (
     (message.source !== MESSAGE_SOURCE.CONSOLE_API &&
       message.source !== MESSAGE_SOURCE.JAVASCRIPT) ||
-    filters[message.level] === true
+    filters[message.level] === true ||
+    (filters[FILTERS.ERROR] && isMessageNetworkError(message))
   );
 }
 
 /**
  * Returns true if the message shouldn't be hidden because of the CSS filter state.
  *
  * @param {Object} message - The message to check the filter against.
  * @param {FilterState} filters - redux "filters" state.
--- a/devtools/client/webconsole/test/browser/browser_webconsole_filters.js
+++ b/devtools/client/webconsole/test/browser/browser_webconsole_filters.js
@@ -9,31 +9,45 @@ const TEST_URI =
   "http://example.com/browser/devtools/client/webconsole/" +
   "test/browser/test-console-filters.html";
 
 add_task(async function() {
   const hud = await openNewTabAndConsole(TEST_URI);
 
   const filterState = await getFilterState(hud);
 
+  // Triggers network requests
+  SpecialPowers.spawn(gBrowser.selectedBrowser, [], () => {
+    const url = "./sjs_slow-response-test-server.sjs";
+    // Set a smaller delay for the 300 to ensure we get it before the "error" responses.
+    content.fetch(`${url}?status=300&delay=100`);
+    content.fetch(`${url}?status=404&delay=500`);
+    content.fetch(`${url}?status=500&delay=500`);
+  });
+
+  // Wait for the messages
+  await waitFor(() => findMessage(hud, "status=404", ".network.error"));
+  await waitFor(() => findMessage(hud, "status=500", ".network.error"));
+
   // Check defaults.
 
   for (const category of ["error", "warn", "log", "info", "debug"]) {
     const state = filterState[category];
     ok(state, `Filter button for ${category} is on by default`);
   }
   for (const category of ["css", "net", "netxhr"]) {
     const state = filterState[category];
     ok(!state, `Filter button for ${category} is off by default`);
   }
 
   // Check that messages are shown as expected. This depends on cached messages being
   // shown.
-  ok(
-    findMessages(hud, "").length == 5,
+  is(
+    findMessages(hud, "").length,
+    7,
     "Messages of all levels shown when filters are on."
   );
 
   // Check that messages are not shown when their filter is turned off.
   await setFilterState(hud, {
     error: false,
   });
   await waitFor(() => findMessages(hud, "").length == 4);
@@ -55,15 +69,16 @@ async function testFilterPersistence() {
     return outputNode.querySelector(".webconsole-filterbar-secondary");
   });
   ok(filterBar, "Filter bar ui setting is persisted.");
   // Check that the filter settings were persisted.
   ok(
     !filterIsEnabled(filterBar.querySelector("[data-category='error']")),
     "Filter button setting is persisted"
   );
-  ok(
-    findMessages(hud, "").length == 4,
-    "testFilterPersistence: Messages of all levels shown when filters are on."
+  is(
+    findMessages(hud, "").length,
+    4,
+    "testFilterPersistence: Messages of all levels but error shown."
   );
 
   await resetFilters(hud);
 }
--- a/devtools/client/webconsole/test/browser/sjs_slow-response-test-server.sjs
+++ b/devtools/client/webconsole/test/browser/sjs_slow-response-test-server.sjs
@@ -1,17 +1,34 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 function handleRequest(request, response) {
   response.processAsync();
 
+  const params = new Map(
+    request.queryString
+      .replace("?", "")
+      .split("&")
+      .map(s => s.split("="))
+  );
+  const delay = params.has("delay") ? params.get("delay") : 300;
+  const status = params.has("status") ? params.get("status") : 200;
+
   let timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
-  timer.initWithCallback(() => {
-    // to avoid garbage collection
-    timer = null;
-    response.setStatusLine(request.httpVersion, 200, "OK");
-    response.setHeader("Content-Type", "text/plain", false);
-    response.setHeader("Set-Cookie", "foo=bar; Max-Age=10; HttpOnly", true);
-    response.write("Some response data");
-    response.finish();
-  }, 300, Ci.nsITimer.TYPE_ONE_SHOT); // Make sure this request takes a few hundred ms.
+  timer.initWithCallback(
+    () => {
+      // to avoid garbage collection
+      timer = null;
+      response.setStatusLine(request.httpVersion, status, "OK");
+      response.setHeader("Content-Type", "text/plain", false);
+      response.setHeader(
+        "Set-Cookie",
+        "foo=bar; Max-Age=10; HttpOnly; SameSite=Lax",
+        true
+      );
+      response.write("Some response data");
+      response.finish();
+    },
+    delay,
+    Ci.nsITimer.TYPE_ONE_SHOT
+  );
 }
--- a/devtools/client/webconsole/test/node/store/filters.test.js
+++ b/devtools/client/webconsole/test/node/store/filters.test.js
@@ -85,17 +85,17 @@ describe("Filtering", () => {
       const messages = getVisibleMessages(store.getState());
       expect(messages.length).toEqual(numUnfilterableMessages + 1);
     });
 
     it("filters error messages", () => {
       store.dispatch(actions.filterToggle(FILTERS.ERROR));
 
       const messages = getVisibleMessages(store.getState());
-      expect(messages.length).toEqual(numUnfilterableMessages + 4);
+      expect(messages.length).toEqual(numUnfilterableMessages + 5);
     });
 
     it("filters css messages", () => {
       const message = stubPreparedMessages.get(
         "Unknown property ‘such-unknown-property’.  Declaration dropped."
       );
       store.dispatch(messagesAdd([message]));
 
@@ -319,16 +319,18 @@ function prepareBaseStore() {
     "ReferenceError: asdf is not defined",
     "TypeError longString message",
     "console.group('bar')",
     "console.debug('debug message');",
     "console.info('info message');",
     "console.error('error message');",
     "console.table(['red', 'green', 'blue']);",
     "console.assert(false, {message: 'foobar'})",
+    // This is a 404 request, it's displayed as an error
+    "GET request",
   ]);
 
   // Console Command - never filtered
   store.dispatch(
     messagesAdd([new ConsoleCommand({ messageText: `console.warn("x")` })])
   );
 
   return store;
--- a/devtools/client/webconsole/test/node/store/search.test.js
+++ b/devtools/client/webconsole/test/node/store/search.test.js
@@ -81,22 +81,22 @@ describe("Searching in grips", () => {
       );
       expect(getVisibleMessages(store.getState()).length).toEqual(0);
     });
   });
 
   describe("Reverse search", () => {
     it("reverse matches on value grips", () => {
       store.dispatch(actions.filterTextSet("-red"));
-      expect(getVisibleMessages(store.getState()).length).toEqual(5);
+      expect(getVisibleMessages(store.getState()).length).toEqual(6);
     });
 
     it("reverse matches on file name", () => {
       store.dispatch(actions.filterTextSet("-test-console-api.html:1:35"));
-      expect(getVisibleMessages(store.getState()).length).toEqual(1);
+      expect(getVisibleMessages(store.getState()).length).toEqual(2);
     });
   });
 });
 
 function prepareBaseStore() {
   const store = setupStore([
     "console.log('foobar', 'test')",
     "console.warn('danger, will robinson!')",
--- a/devtools/client/webconsole/utils/messages.js
+++ b/devtools/client/webconsole/utils/messages.js
@@ -754,25 +754,34 @@ function getNaturalOrder(messageA, messa
       return parseInt(messageA.id, 10) < parseInt(messageB.id, 10)
         ? aFirst
         : bFirst;
     }
   }
   return messageA.timeStamp < messageB.timeStamp ? aFirst : bFirst;
 }
 
+function isMessageNetworkError(message) {
+  return (
+    message.source === MESSAGE_SOURCE.NETWORK &&
+    message?.response?.status &&
+    message.response.status.toString().match(/^[4,5]\d\d$/)
+  );
+}
+
 module.exports = {
   createWarningGroupMessage,
   getArrayTypeNames,
   getDescriptorValue,
   getInitialMessageCountForViewport,
   getNaturalOrder,
   getParentWarningGroupMessageId,
   getWarningGroupType,
   isContentBlockingMessage,
   isGroupType,
+  isMessageNetworkError,
   isPacketPrivate,
   isWarningGroup,
   l10n,
   prepareMessage,
   // Export for use in testing.
   getRepeatId,
 };