Bug 1581963 - [devtools] Handle errors properly when capturing a profile r=profiler-reviewers,mstange

Differential Revision: https://phabricator.services.mozilla.com/D245886

devtools/client/performance-new/@types/perf.d.ts

@@ -215,7 +215,8 @@ export interface SymbolicationService {
  * profile has been obtained.
  */
 export type OnProfileReceived = (
-  profileAndAdditionalInformation: ProfileAndAdditionalInformation
+  profileAndAdditionalInformation: ProfileAndAdditionalInformation | null,
+  error?: Error | string
 ) => void;
 
 /**
@@ -643,7 +644,7 @@ export class ProfilerWebChannel {
  */
 export type ProfilerBrowserInfo = {
   profileCaptureResult: ProfileCaptureResult;
-  symbolicationService: SymbolicationService;
+  symbolicationService: SymbolicationService | null;
 };
 
 export type ProfileCaptureResult =

devtools/client/performance-new/components/panel/RecordingButton.js

@@ -75,9 +75,14 @@ class RecordingButton extends PureComponent {
   _onCaptureButtonClick = async () => {
     const { getProfileAndStopProfiler, onProfileReceived, perfFront } =
       this.props;
+    try {
       const profileAndAdditionalInformation =
         await getProfileAndStopProfiler(perfFront);
       onProfileReceived(profileAndAdditionalInformation);
+    } catch (e) {
+      const assertedError = /** @type {Error | string} */ (e);
+      onProfileReceived(null, assertedError);
+    }
   };
 
   _onStopButtonClick = () => {

devtools/client/performance-new/panel/initializer.js

@@ -131,11 +131,31 @@ async function gInit(perfFront, traits, pageContext, openAboutProfiling) {
   );
 
   /**
-   * @param {MockedExports.ProfileAndAdditionalInformation} profileAndAdditionalInformation
+   * @param {MockedExports.ProfileAndAdditionalInformation | null} profileAndAdditionalInformation
+   * @param {Error | string} [error]
    */
-  const onProfileReceived = async ({ profile, additionalInformation }) => {
+  const onProfileReceived = async (profileAndAdditionalInformation, error) => {
     const objdirs = selectors.getObjdirs(store.getState());
     const profilerViewMode = getProfilerViewModeForCurrentPreset(pageContext);
+    const browser = await openProfilerTab({ profilerViewMode });
+
+    if (error || !profileAndAdditionalInformation) {
+      if (!error) {
+        error =
+          "No profile data has been passed to onProfileReceived, and no specific error has been specified. This is unexpected.";
+      }
+      /**
+       * @type {ProfileCaptureResult}
+       */
+      const profileCaptureResult = {
+        type: "ERROR",
+        error: typeof error === "string" ? new Error(error) : error,
+      };
+      registerProfileCaptureForBrowser(browser, profileCaptureResult, null);
+      return;
+    }
+
+    const { profile, additionalInformation } = profileAndAdditionalInformation;
     const sharedLibraries = additionalInformation?.sharedLibraries ?? [];
     if (!sharedLibraries.length) {
       console.error(
@@ -147,7 +167,6 @@ async function gInit(perfFront, traits, pageContext, openAboutProfiling) {
       objdirs,
       perfFront
     );
-    const browser = await openProfilerTab({ profilerViewMode });
 
     /**
      * @type {ProfileCaptureResult}

devtools/client/performance-new/shared/background.sys.mjs

@@ -289,11 +289,17 @@ async function getResponseForMessage(request, browser) {
     case "GET_SYMBOL_TABLE": {
       const { debugName, breakpadId } = request;
       const symbolicationService = getSymbolicationServiceForBrowser(browser);
+      if (!symbolicationService) {
+        throw new Error("No symbolication service has been found for this tab");
+      }
       return symbolicationService.getSymbolTable(debugName, breakpadId);
     }
     case "QUERY_SYMBOLICATION_API": {
       const { path, requestJson } = request;
       const symbolicationService = getSymbolicationServiceForBrowser(browser);
+      if (!symbolicationService) {
+        throw new Error("No symbolication service has been found for this tab");
+      }
       return symbolicationService.querySymbolicationApi(path, requestJson);
     }
     case "GET_EXTERNAL_POWER_TRACKS": {
@@ -351,7 +357,7 @@ async function getResponseForMessage(request, browser) {
  * tab, or a fallback service for browsers from tabs opened by the user.
  *
  * @param {MockedExports.Browser} browser
- * @return {SymbolicationService}
+ * @return {SymbolicationService | null}
  */
 function getSymbolicationServiceForBrowser(browser) {
   // We try to serve symbolication requests that come from tabs that we
@@ -429,7 +435,7 @@ export async function handleWebChannelMessage(channel, id, message, target) {
 /**
  * @param {MockedExports.Browser} browser - The tab's browser.
  * @param {ProfileCaptureResult} profileCaptureResult - The Gecko profile.
- * @param {SymbolicationService} symbolicationService - An object which implements the
+ * @param {SymbolicationService | null} symbolicationService - An object which implements the
  *   SymbolicationService interface, whose getSymbolTable method will be invoked
  *   when profiler.firefox.com sends GET_SYMBOL_TABLE WebChannel messages to us. This
  *   method should obtain a symbol table for the requested binary and resolve the

devtools/server/tests/browser/browser_perf-01.js

@@ -78,7 +78,26 @@ add_task(async function () {
     "The profiler was stopped and the profile discarded."
   );
 
-  // Clean up.
+  await front.destroy();
+  await client.close();
+});
+
+add_task(async function test_error_case() {
+  const { front, client } = await initPerfFront();
+
+  try {
+    // We try to get the profile without starting the profiler first. This should
+    // trigger an error in the our C++ code.
+    await front.getProfileAndStopProfiler();
+    ok(false, "Getting the profile should fail");
+  } catch (e) {
+    Assert.stringContains(
+      e.message,
+      "The profiler is not active.",
+      "The error contains the expected error message."
+    );
+  }
+
   await front.destroy();
   await client.close();
 });