Skip to content

fix: prevent telemetry interceptor from masking transport errors in extendToM365#15782

Closed
Alive-Fish wants to merge 2 commits intorelease/6.9from
zhiyou/fix-extendToM365-error-masking
Closed

fix: prevent telemetry interceptor from masking transport errors in extendToM365#15782
Alive-Fish wants to merge 2 commits intorelease/6.9from
zhiyou/fix-extendToM365-error-masking

Conversation

@Alive-Fish
Copy link
Copy Markdown
Contributor

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 5 regression tests:
    • Transport-level error with request.method undefined → no throw.
    • Error with no request object → no throw.
    • MOS API error with string response.data → no throw.
    • convertUrlToApiName(_, undefined) → no throw.
    • convertMethodUrlToApiDefForMOS(undefined, _) → no throw.

Testing

All 20 wrappedAxiosClient.test.ts tests pass locally.

Bug: AB#37640864
Related: #15676

…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)
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.

1 participant