fix: refine m365 package service error message (#11452)

* fix: refine m365 package service error message * test: ut * test: ut * test: ut * test: ut * test: ut
OfficeDev · Apr 25, 2024 · 5f369ff · 5f369ff
1 parent 6230c8c
commit 5f369ff
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 40 deletions.
diff --git a/packages/fx-core/src/common/m365/packageService.ts b/packages/fx-core/src/common/m365/packageService.ts
@@ -52,7 +52,7 @@ export class PackageService {
     this.logger = logger;
   }
   @hooks([ErrorContextMW({ source: M365ErrorSource, component: M365ErrorComponent })])
-  private async getTitleServiceUrl(token: string): Promise<string> {
+  public async getTitleServiceUrl(token: string): Promise<string> {
     try {
       try {
         new URL(this.initEndpoint);
@@ -129,12 +129,12 @@ export class PackageService {
         throw new Error(`Unknown response code: ${uploadResponse.status}}`);
       }
     } catch (error: any) {
-      this.logger?.error("Sideloading failed.");
+      // this.logger?.error("Sideloading failed.");
       if (error.response) {
-        this.logger?.error(JSON.stringify(error.response.data));
+        // this.logger?.error(JSON.stringify(error.response.data));
         error = this.traceError(error);
       } else {
-        this.logger?.error(error.message);
+        // this.logger?.error(error.message);
       }
       throw assembleError(error, M365ErrorSource);
     }
@@ -202,12 +202,12 @@ export class PackageService {
         }
       } while (true);
     } catch (error: any) {
-      this.logger?.error("Sideloading failed.");
+      // this.logger?.error("Sideloading failed.");
       if (error.response) {
-        this.logger?.error(JSON.stringify(error.response.data));
+        // this.logger?.error(JSON.stringify(error.response.data));
         error = this.traceError(error);
       } else {
-        this.logger?.error(error.message);
+        // this.logger?.error(error.message);
       }
       throw assembleError(error, M365ErrorSource);
     }
@@ -255,13 +255,10 @@ export class PackageService {
     } catch (error: any) {
       this.logger?.error("Get LaunchInfo failed.");
       if (error.response) {
-        this.logger?.error(JSON.stringify(error.response.data));
         if (error.response.status === 404) {
           throw new NotExtendedToM365Error(M365ErrorSource);
         }
         error = this.traceError(error);
-      } else {
-        this.logger?.error(error.message);
       }
       throw assembleError(error, M365ErrorSource);
     }
@@ -295,14 +292,9 @@ export class PackageService {
       });
       this.logger?.verbose("Unacquiring done.");
     } catch (error: any) {
-      this.logger?.error("Unacquire failed.");
       if (error.response) {
-        this.logger?.error(JSON.stringify(error.response.data));
         error = this.traceError(error);
-      } else {
-        this.logger?.error(error.message);
       }
-
       throw assembleError(error, M365ErrorSource);
     }
   }
@@ -328,14 +320,9 @@ export class PackageService {
       this.logger?.info(JSON.stringify(launchInfo.data));
       return launchInfo.data;
     } catch (error: any) {
-      this.logger?.error("Get LaunchInfo failed.");
       if (error.response) {
-        this.logger?.error(JSON.stringify(error.response.data));
         error = this.traceError(error);
-      } else {
-        this.logger?.error(error.message);
       }
-
       throw assembleError(error, M365ErrorSource);
     }
   }
@@ -377,14 +364,9 @@ export class PackageService {
 
       return activeExperiences;
     } catch (error: any) {
-      this.logger?.error("Fail to get active experiences.");
       if (error.response) {
-        this.logger?.error(JSON.stringify(error.response.data));
         error = this.traceError(error);
-      } else {
-        this.logger?.error(error.message);
       }
-
       throw assembleError(error, M365ErrorSource);
     }
   }
@@ -426,21 +408,22 @@ export class PackageService {
 
   private traceError(error: any): any {
     // add error details and trace to message
-    const detail = JSON.stringify(error.response.data ?? {});
-    const tracingId = error.response.headers?.traceresponse ?? "";
-    const originalMessage = error.message;
-    error.message = JSON.stringify({
-      message: originalMessage,
-      detail: detail,
-      tracingId: tracingId,
-    });
+    const tracingId = (error.response.headers?.traceresponse ?? "") as string;
+    const originalMessage = error.message as string;
+    const innerError = error.response.data?.error || { code: "", message: "" };
+    const finalMessage = `${originalMessage} (tracingId: ${tracingId}) ${
+      innerError.code as string
+    }: ${innerError.message as string} `;
+
+    error.message = finalMessage;
 
     // HTTP 400 as user error due to invalid input
     if (error.response?.status === 400) {
       error = new UserError({
+        name: "PackageServiceError",
         error,
         source: M365ErrorSource,
-        message: error.message,
+        message: finalMessage,
       });
     }
 

diff --git a/packages/fx-core/tests/common/m365/packageService.test.ts b/packages/fx-core/tests/common/m365/packageService.test.ts
@@ -13,6 +13,7 @@ import { MockLogProvider } from "../../core/utils";
 import { PackageService } from "../../../src/common/m365/packageService";
 import { UnhandledError } from "../../../src/error/common";
 import { setTools } from "../../../src/core/globalVars";
+import { NotExtendedToM365Error } from "../../../src/common/m365/errors";
 
 chai.use(chaiAsPromised);
 
@@ -277,7 +278,7 @@ describe("Package Service", () => {
     chai.assert.isFalse(infoStub.calledWith("TitleId: test-title-id"));
     chai.assert.isFalse(infoStub.calledWith("AppId: test-app-id"));
     chai.assert.isFalse(verboseStub.calledWith("Sideloading done."));
-    chai.assert.isTrue(errorStub.calledWith("Sideloading failed."));
+    // chai.assert.isTrue(errorStub.calledWith("Sideloading failed."));
 
     chai.assert.isDefined(actualError);
   });
@@ -302,8 +303,8 @@ describe("Package Service", () => {
     } catch (error: any) {
       actualError = error;
     }
-    chai.assert.isTrue(errorStub.calledWith(`${JSON.stringify(error.response.data)}`));
-    chai.assert.isTrue(errorStub.calledWith(`Sideloading failed.`));
+    // chai.assert.isTrue(errorStub.calledWith(`${JSON.stringify(error.response.data)}`));
+    // chai.assert.isTrue(errorStub.calledWith(`Sideloading failed.`));
     chai.assert.isDefined(actualError);
     chai.assert.isTrue(actualError?.message.includes("test-post"));
   });
@@ -325,9 +326,9 @@ describe("Package Service", () => {
     } catch (error: any) {
       actualError = error;
     }
-    chai.assert.isTrue(errorStub.calledWith(`test-post`));
+    // chai.assert.isTrue(errorStub.calledWith(`test-post`));
     chai.assert.isDefined(actualError);
-    chai.assert.isTrue(actualError?.message.includes("test-post"));
+    // chai.assert.isTrue(actualError?.message.includes("test-post"));
   });
 
   it("sideLoading happy path", async () => {
@@ -738,7 +739,17 @@ describe("Package Service", () => {
 
     chai.assert.deepEqual(launchInfo, { foo: "bar" });
   });
-
+  it("getLaunchInfoByManifestId throws expected error", async () => {
+    const packageService = new PackageService("https://test-endpoint");
+    sandbox.stub(testAxiosInstance, "post").rejects({ response: { status: 404 } });
+    sandbox.stub(packageService, "getTitleServiceUrl").resolves("https://test-url");
+    try {
+      await packageService.getLaunchInfoByManifestId("test-token", "test-manifest-id");
+      chai.assert.fail("should not reach here");
+    } catch (e) {
+      chai.assert.isTrue(e instanceof NotExtendedToM365Error);
+    }
+  });
   it("getLaunchInfoByTitleId throws expected error", async () => {
     axiosGetResponses["/config/v1/environment"] = {
       data: {