fix: prevent telemetry interceptor from masking transport errors in extendToM365 (cherry-pick to dev)

packages/fx-core/src/common/wrappedAxiosClient.ts

@@ -90,69 +90,84 @@ export class WrappedAxiosClient {
    * @returns
    */
   public static onRejected(error: AxiosError) {
-    const method = error.request.method as string;
-    const fullPath = `${(error.request.host as string) ?? ""}${
-      (error.request.path as string) ?? ""
-    }`;
-    const apiName = this.convertUrlToApiName(fullPath, method);
+    // Telemetry must never throw, otherwise the synthetic error will mask the
+    // real transport-level failure (TLS handshake, ECONNRESET on a kept-alive
+    // socket, etc.) returned to the caller. See AB#37640864.
+    try {
+      const method = ((error.request?.method as string) ?? "").toString();
+      const fullPath = `${(error.request?.host as string) ?? ""}${
+        (error.request?.path as string) ?? ""
+      }`;
+      const apiName = this.convertUrlToApiName(fullPath, method);
 
-    let requestData: any;
-    if (error.config?.data && typeof error.config.data === "string") {
-      try {
-        requestData = JSON.parse(error.config.data);
-      } catch (error) {
-        requestData = undefined;
+      let requestData: any;
+      if (error.config?.data && typeof error.config.data === "string") {
+        try {
+          requestData = JSON.parse(error.config.data);
+        } catch (error) {
+          requestData = undefined;
+        }
       }
-    }
-    const properties: { [key: string]: string } = {
-      url: `<${apiName}-url>`,
-      method: method,
-      params: this.generateParameters(error.config!.params),
-      [TelemetryProperty.Success]: TelemetrySuccess.No,
-      [TelemetryProperty.ErrorMessage]: error.response
-        ? JSON.stringify(error.response.data)
-        : error.message ?? "undefined",
-      "status-code": error.response?.status.toString() ?? "undefined",
-      ...this.generateExtraProperties(fullPath, requestData),
-    };
+      const properties: { [key: string]: string } = {
+        url: `<${apiName}-url>`,
+        method: method,
+        params: this.generateParameters(error.config?.params),
+        [TelemetryProperty.Success]: TelemetrySuccess.No,
+        [TelemetryProperty.ErrorMessage]: error.response
+          ? JSON.stringify(error.response.data)
+          : error.message ?? "undefined",
+        "status-code": error.response?.status.toString() ?? "undefined",
+        ...this.generateExtraProperties(fullPath, requestData),
+      };
 
-    const eventName = this.getEventName(fullPath);
-    if (eventName === TelemetryEvent.AppStudioApi) {
-      const correlationId = error.response?.headers[Constants.CORRELATION_ID] ?? "undefined";
+      const eventName = this.getEventName(fullPath);
+      if (eventName === TelemetryEvent.AppStudioApi) {
+        const correlationId =
+          (error.response?.headers
+            ? error.response.headers[Constants.CORRELATION_ID]
+            : undefined) ?? "undefined";
 
-      const extraData = getDefaultString(
-        "error.appstudio.apiFailed.reason.common",
-        error.response?.data ? `data: ${JSON.stringify(error.response.data)}` : ""
-      );
-      const TDPApiFailedError = new DeveloperPortalAPIFailedSystemError(
-        error,
-        correlationId,
-        apiName,
-        extraData
-      );
-      properties[TelemetryProperty.ErrorCode] =
-        `${TDPApiFailedError.source}.${TDPApiFailedError.name}`;
-      properties[TelemetryProperty.ErrorMessage] = TDPApiFailedError.message;
-      properties[TelemetryProperty.TDPTraceId] = correlationId;
-    } else if (eventName === TelemetryEvent.MOSApi) {
-      const tracingId = (error.response?.headers?.traceresponse ?? "undefined") as string;
-      const originalMessage = error.message;
-      const innerError = (error.response?.data as any).error || { code: "", message: "" };
-      const finalMessage = `${originalMessage} (tracingId: ${tracingId}) ${
-        innerError.code as string
-      }: ${innerError.message as string} `;
-      properties[TelemetryProperty.ErrorMessage] = finalMessage;
-      properties[TelemetryProperty.MOSTraceId] = tracingId;
-    }
+        const extraData = getDefaultString(
+          "error.appstudio.apiFailed.reason.common",
+          error.response?.data ? `data: ${JSON.stringify(error.response.data)}` : ""
+        );
+        const TDPApiFailedError = new DeveloperPortalAPIFailedSystemError(
+          error,
+          correlationId as string,
+          apiName,
+          extraData
+        );
+        properties[TelemetryProperty.ErrorCode] =
+          `${TDPApiFailedError.source}.${TDPApiFailedError.name}`;
+        properties[TelemetryProperty.ErrorMessage] = TDPApiFailedError.message;
+        properties[TelemetryProperty.TDPTraceId] = correlationId as string;
+      } else if (eventName === TelemetryEvent.MOSApi) {
+        const tracingId = (error.response?.headers?.traceresponse ?? "undefined") as string;
+        const originalMessage = error.message;
+        const responseData = error.response?.data;
+        const innerError =
+          responseData && typeof responseData === "object"
+            ? (responseData as any).error ?? { code: "", message: "" }
+            : { code: "", message: "" };
+        const finalMessage = `${originalMessage} (tracingId: ${tracingId}) ${
+          (innerError.code as string) ?? ""
+        }: ${(innerError.message as string) ?? ""} `;
+        properties[TelemetryProperty.ErrorMessage] = finalMessage;
+        properties[TelemetryProperty.MOSTraceId] = tracingId;
+      }
 
-    TOOLS?.telemetryReporter?.sendTelemetryErrorEvent(eventName, properties);
+      TOOLS?.telemetryReporter?.sendTelemetryErrorEvent(eventName, properties);
+    } catch {
+      // Swallow telemetry errors so we always reject with the original error.
+    }
     return Promise.reject(error);
   }
 
   static convertMethodUrlToApiDefForMOS(method: string, url: string): MOS3Api | undefined {
+    const upperMethod = (method ?? "").toUpperCase();
     for (const key of Object.keys(MOS3ApiDefinitions)) {
       const api = MOS3ApiDefinitions[key];
-      if (api.method === method.toUpperCase() && url.match(api.path)) {
+      if (api.method === upperMethod && url.match(api.path)) {
         return api;
       }
     }
@@ -168,7 +183,7 @@ export class WrappedAxiosClient {
    * @returns
    */
   public static convertUrlToApiName(fullPath: string, method: string): string {
-    const upperMethod = method.toUpperCase();
+    const upperMethod = (method ?? "").toUpperCase();
 
     if (this.isTDPApi(fullPath)) {
       if (fullPath.match(new RegExp("/api/aadapp/v2"))) {

packages/fx-core/tests/common/wrappedAxiosClient.test.ts

@@ -288,6 +288,201 @@ describe("Wrapped Axios Client Test", () => {
     chai.expect(telemetryChecker.calledOnce).to.be.true;
   });
 
+  // Regression tests for AB#37640864: telemetry must never throw / mask the
+  // real transport-level error. Triggered by retry + keepAlive in PR #15676.
+  it("transport-level error with undefined request.method does not throw", async () => {
+    const transportError = {
+      message: "socket hang up",
+      code: "ECONNRESET",
+      request: {
+        // method, host, path can all be undefined for low-level socket errors
+        host: "https://titles.prod.mos.microsoft.com",
+        path: "/dev/v1/users/packages",
+      },
+      config: {},
+      // no `response` property -> transport failure
+    } as any;
+    const telemetryChecker = sinon.spy(mockTools.telemetryReporter, "sendTelemetryErrorEvent");
+
+    let rejected: any;
+    await WrappedAxiosClient.onRejected(transportError).catch((e) => (rejected = e));
+
+    chai.expect(rejected).to.equal(transportError);
+    chai.expect(telemetryChecker.calledOnce).to.be.true;
+  });
+
+  it("error with no request object does not throw", async () => {
+    const transportError = {
+      message: "TLS handshake failed",
+      code: "EPROTO",
+      config: {},
+    } as any;
+
+    let rejected: any;
+    await WrappedAxiosClient.onRejected(transportError).catch((e) => (rejected = e));
+
+    chai.expect(rejected).to.equal(transportError);
+  });
+
+  it("MOS API error with non-object response.data does not throw", async () => {
+    const mockedError = {
+      message: "Bad Gateway",
+      request: {
+        method: "POST",
+        host: "https://titles.prod.mos.microsoft.com",
+        path: "/dev/v1/users/packages",
+      },
+      config: {},
+      response: {
+        status: 502,
+        // data is a string (e.g. HTML body from a gateway), not an object
+        data: "<html>502 Bad Gateway</html>",
+        headers: { traceresponse: "trace-123" },
+      },
+    } as any;
+    const telemetryChecker = sinon.spy(mockTools.telemetryReporter, "sendTelemetryErrorEvent");
+
+    let rejected: any;
+    await WrappedAxiosClient.onRejected(mockedError).catch((e) => (rejected = e));
+
+    chai.expect(rejected).to.equal(mockedError);
+    chai.expect(telemetryChecker.calledOnce).to.be.true;
+  });
+
+  it("convertUrlToApiName handles undefined method", () => {
+    const apiName = WrappedAxiosClient.convertUrlToApiName(
+      "https://example.com/foo",
+      undefined as any
+    );
+    chai.expect(apiName).to.be.a("string");
+  });
+
+  it("convertMethodUrlToApiDefForMOS handles undefined method", () => {
+    const result = WrappedAxiosClient.convertMethodUrlToApiDefForMOS(
+      undefined as any,
+      "https://example.com/foo"
+    );
+    chai.expect(result).to.be.undefined;
+  });
+
+  it("TDP API error response without headers does not throw", async () => {
+    const mockedError = {
+      message: "Bad Request",
+      request: {
+        method: "GET",
+        host: getResourceServiceEndpoint(ResourceServiceType.TDP),
+        path: "/api/appdefinitions/fakeId",
+      },
+      config: {},
+      response: {
+        status: 400,
+        // headers intentionally omitted
+      },
+    } as any;
+    const telemetryChecker = sinon.spy(mockTools.telemetryReporter, "sendTelemetryErrorEvent");
+
+    let rejected: any;
+    await WrappedAxiosClient.onRejected(mockedError).catch((e) => (rejected = e));
+
+    chai.expect(rejected).to.equal(mockedError);
+    chai.expect(telemetryChecker.calledOnce).to.be.true;
+  });
+
+  it("MOS API error with nested response.data.error is surfaced", async () => {
+    const mockedError = {
+      message: "Conflict",
+      request: {
+        method: "POST",
+        host: "https://titles.prod.mos.microsoft.com",
+        path: "/dev/v1/users/packages",
+      },
+      config: {},
+      response: {
+        status: 409,
+        data: {
+          error: { code: "Conflict", message: "Already exists" },
+        },
+        headers: { traceresponse: "trace-xyz" },
+      },
+    } as any;
+    const telemetryChecker = sinon.spy(mockTools.telemetryReporter, "sendTelemetryErrorEvent");
+
+    let rejected: any;
+    await WrappedAxiosClient.onRejected(mockedError).catch((e) => (rejected = e));
+
+    chai.expect(rejected).to.equal(mockedError);
+    chai.expect(telemetryChecker.calledOnce).to.be.true;
+    const props = telemetryChecker.firstCall.args[1] as any;
+    chai.expect(props["err-message"]).to.contain("Conflict");
+    chai.expect(props["err-message"]).to.contain("Already exists");
+    chai.expect(props["err-message"]).to.contain("trace-xyz");
+  });
+
+  it("onRejected swallows internal telemetry errors and still rejects", async () => {
+    const mockedError = {
+      message: "boom",
+      request: { method: "GET", host: "https://example.com", path: "/x" },
+      config: {},
+    } as any;
+    // Force the telemetry reporter itself to throw, exercising the outer catch.
+    sinon
+      .stub(mockTools.telemetryReporter, "sendTelemetryErrorEvent")
+      .throws(new Error("telemetry exploded"));
+
+    let rejected: any;
+    await WrappedAxiosClient.onRejected(mockedError).catch((e) => (rejected = e));
+
+    chai.expect(rejected).to.equal(mockedError);
+  });
+
+  it("onRejected handles minimal error shape with no config / no message", async () => {
+    // Bare-minimum error object: no config, no message, no response.
+    // Exercises the "?? undefined" / "?? '...'" nullish fallback branches.
+    const mockedError = {
+      request: {
+        host: "https://titles.prod.mos.microsoft.com",
+        path: "/dev/v1/users/packages",
+      },
+    } as any;
+    const telemetryChecker = sinon.spy(mockTools.telemetryReporter, "sendTelemetryErrorEvent");
+
+    let rejected: any;
+    await WrappedAxiosClient.onRejected(mockedError).catch((e) => (rejected = e));
+
+    chai.expect(rejected).to.equal(mockedError);
+    chai.expect(telemetryChecker.calledOnce).to.be.true;
+  });
+
+  it("MOS API error with response.data.error missing fields uses fallback", async () => {
+    const mockedError = {
+      message: "Server Error",
+      request: {
+        method: "POST",
+        host: "https://titles.prod.mos.microsoft.com",
+        path: "/dev/v1/users/packages",
+      },
+      config: {},
+      response: {
+        status: 500,
+        // .error exists but has neither .code nor .message → exercises the
+        // `(innerError.code as string) ?? ""` and `... ?? ""` fallbacks.
+        data: { error: {} },
+        // no `headers.traceresponse` → exercises tracingId "undefined" fallback
+        headers: {},
+      },
+    } as any;
+    const telemetryChecker = sinon.spy(mockTools.telemetryReporter, "sendTelemetryErrorEvent");
+
+    let rejected: any;
+    await WrappedAxiosClient.onRejected(mockedError).catch((e) => (rejected = e));
+
+    chai.expect(rejected).to.equal(mockedError);
+    chai.expect(telemetryChecker.calledOnce).to.be.true;
+    const props = telemetryChecker.firstCall.args[1] as any;
+    chai.expect(props["err-message"]).to.contain("Server Error");
+    chai.expect(props["err-message"]).to.contain("undefined"); // tracingId fallback
+  });
+
   it("Create bot API start telemetry", async () => {
     const mockedRequest = {
       method: "POST",