Skip to content

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

Merged
Alive-Fish merged 2 commits intodevfrom
zhiyou/fix-extendToM365-error-masking-dev
Apr 23, 2026
Merged

fix: prevent telemetry interceptor from masking transport errors in extendToM365 (cherry-pick to dev)#15785
Alive-Fish merged 2 commits intodevfrom
zhiyou/fix-extendToM365-error-masking-dev

Conversation

@Alive-Fish
Copy link
Copy Markdown
Contributor

Cherry-pick of #15782 (release/6.9 → dev).

Summary

Fixes AB#37640864.

When provision runs teamsApp/extendToM365 under network latency, the user sees a misleading error:

Cannot read properties of undefined (reading 'toUpperCase')

instead of the real MOS/network failure (TLS handshake, ECONNRESET, "socket hang up", etc.).

Root cause

The Axios telemetry interceptor WrappedAxiosClient.onRejected calls convertUrlToApiName(fullPath, method), which does method.toUpperCase(). For low-level transport errors, error.request.method is often undefined, so the interceptor itself throws a synthetic TypeError that propagates out of PackageService and replaces the real error.

PR #15676 (keepAlive + 3-attempt retry on extendToM365) is what surfaced this latent bug:

  • keepAlive: true reuses sockets that the server may have silently closed, producing exactly the "method undefined" class of AxiosError.
  • After 3 retries, the final transport error hits the interceptor and the masking TypeError is thrown — matching the bug report's "failed by retry 3 times".

Changes

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

  • Wrap entire onRejected body in try { … } catch {} so telemetry can never replace the original error.
  • Guard error.request?.method/host/path.
  • Guard method.toUpperCase() in convertUrlToApiName and convertMethodUrlToApiDefForMOS.
  • Tolerate non-object error.response.data (e.g. HTML 502 body) in the MOS branch.
  • Make error.response.headers[CORRELATION_ID] lookup safe when headers is missing.

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

  • Add 10 regression tests covering: undefined request.method, missing request object, string response.data, AppStudio TDP error without headers, MOS error with nested response.data.error, MOS error with empty inner error fields, telemetry-reporter throwing (outer try/catch), and minimal error shape.

Testing

All 25 wrappedAxiosClient.test.ts tests pass. Branch coverage on the changed file: 95.9%; line coverage: 100%.

Bug: AB#37640864
Related: #15676, #15782

…xtendToM365

- Wrap WrappedAxiosClient.onRejected in try/catch so any defect in
  telemetry can never replace the real transport-level error returned
  to the caller.
- Guard error.request?.method/host/path - transport errors (TLS
  handshake, ECONNRESET on a kept-alive socket, socket hang up) can
  have these undefined.
- Guard method.toUpperCase() in convertUrlToApiName and
  convertMethodUrlToApiDefForMOS.
- Tolerate non-object error.response.data (e.g. HTML 502 body) in the
  MOS branch.
- Add 5 regression tests covering undefined method, missing request
  object, and string response.data.

Bug: AB#37640864
Related: #15676 (keepAlive + retry exposed this latent bug)
Adds 5 more regression tests exercising:
- AppStudio TDP error without headers
- MOS error with nested response.data.error
- MOS error with empty inner error fields
- telemetry-reporter throwing (exercises outer try/catch)
- minimal error shape with no config / no message

Brings src/common/wrappedAxiosClient.ts branch coverage from ~92.6%
to 95.9% and line coverage to 100% for the changed code.
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 23, 2026

Codecov Report

❌ Patch coverage is 97.29730% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 93.31%. Comparing base (527d55b) to head (3d1adb8).
⚠️ Report is 2 commits behind head on dev.

Files with missing lines Patch % Lines
packages/fx-core/src/common/wrappedAxiosClient.ts 97.29% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              dev   #15785      +/-   ##
==========================================
+ Coverage   93.29%   93.31%   +0.01%     
==========================================
  Files         585      585              
  Lines       37006    37012       +6     
  Branches     6928     6934       +6     
==========================================
+ Hits        34526    34536      +10     
  Misses       1836     1836              
+ Partials      644      640       -4     
Files with missing lines Coverage Δ
packages/fx-core/src/common/wrappedAxiosClient.ts 98.01% <97.29%> (+2.61%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Alive-Fish Alive-Fish merged commit d3d38ed into dev Apr 23, 2026
51 of 54 checks passed
@Alive-Fish Alive-Fish deleted the zhiyou/fix-extendToM365-error-masking-dev branch April 23, 2026 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants