Skip to content

Surface nested aggregate exception details - #2307

Draft
rksharma-owg wants to merge 2 commits into
cisagov:mainfrom
rksharma-owg:codex/2112-preserve-aggregate-exceptions
Draft

rksharma-owg wants to merge 2 commits into
cisagov:mainfrom
rksharma-owg:codex/2112-preserve-aggregate-exceptions

Conversation

@rksharma-owg

@rksharma-owg rksharma-owg commented Jul 30, 2026 •

Copy link
Copy Markdown

🗣 Description

  • Add Get-FullExceptionDetails as a shared utility for preserving every nested AggregateException message and the script stack trace.
  • Use the shared details in Connect-Tenant and CommandTracker.TryCommand warnings and structured log data.
  • Add regression coverage for tenant authentication, provider commands, non-aggregate exceptions, multiple aggregate branches, and nested inner exceptions.

💭 Motivation and context

Problem

The affected catch blocks only read the top-level Exception.Message. When an asynchronous cmdlet throws an AggregateException, the actionable provider or authentication error can remain nested, leaving users with a generic diagnostic such as One or more errors occurred.

Root cause

Neither catch block traversed the aggregate exception tree before formatting its console warning or log data.

Solution

The new utility collects the top-level message, walks all aggregate branches and nested inner exceptions in deterministic breadth-first order, and returns that message set with the existing script stack trace. Non-aggregate exception formatting remains unchanged. The connection and provider paths now share this implementation instead of duplicating exception traversal.

The original wrong-Teams-AppId reproduction no longer reaches this path after #2342. The current regression tests instead inject an AggregateException at the present authentication/provider boundaries and assert the nested message appears as its own warning line.

Resolves #2112.

🧪 Testing

Exact patch: 8d9b4bf999147769dfef34b2a62ffee8f1f875d8
Current baseline: 6018b15178ae7dd51edcc7b1e6608a2d4411f41d

Local environment: macOS 27.0 (arm64), PowerShell 7.6.6, Pester 5.7.1, PSScriptAnalyzer 1.25.0.

  • Focused contribution regressions plus the Update-ScubaGear suite: 80 passed, 0 failed, 0 skipped.
  • Strict changed-file PSScriptAnalyzer with repository settings: 0 findings on all six files. A transient analyzer error on the first invocation did not count as a pass; the strict retry completed successfully.
  • git diff --check against current main: passed.
  • Fork CI Pipeline on the exact patch: all seven jobs passed, including YAML and PowerShell lint, Markdown syntax, Gitleaks, Checkov, Windows Pester, and OPA/Regal.
  • Fork Workflow Pipeline on the same exact patch: all seven jobs passed, including 94 workflow tests and 3 action tests.
  • Each hosted Windows Pester run executed the full suite: 1,725 passed, 0 failed, 0 skipped.
  • OPA strict checks and all 1,063 tests passed. The upstream-pinned Regal 0.42.0 reported 0 errors and 3 existing warnings.
  • Current-main baseline CI passed all seven jobs: 1,721 Windows Pester tests, 1,063 OPA tests, and Regal lint with the same existing warnings.

The branch includes current main, which contains the accepted version-fixture repair from #2446 and the upstream Regal pin. This PR adds no unrelated test-fixture or workflow changes. Local extended version tests encountered five WindowsPrincipal platform failures on macOS; those tests passed in the complete hosted Windows suites. Upstream-only approval, smoke, and tenant-functional checks are not claimed as passed.

Risk

  • Compatibility and control-flow risk are low: command success/failure handling and return values are unchanged.
  • Aggregate failures expose more vendor-provided diagnostic text in the existing console and log channels, which is the behavior requested in Connect-Tenant hides the underlying authentication exception in some scenarios #2112. Existing debug-report redaction remains in place.
  • No dependencies, external calls, telemetry, or persistent data formats are added.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • PR targets the correct parent branch (main) for merge.
  • Changes are limited to a single goal.
  • Changes are sized such that they do not touch an excessive number of files.
  • No future TODOs are introduced.
  • These code changes follow the ScubaGear content style guide.
  • The related issue is linked with a closing keyword.
  • Unit tests cover the PowerShell changes.
  • All automated checks passed. Fork validation passed; upstream PR workflows still require approval.

✅ Pre-merge checklist

  • PR passed smoke test check.
  • Feature branch passes functional tests for relevant products, if applicable.
  • Feature branch includes current parent-branch changes, merged without rewriting history.
  • Resolved all merge conflicts on branch.
  • Squash all commits into one PR-level commit using the Squash and merge button.

✅ Post-merge checklist

  • Feature branch deleted after merge.
  • Close issues resolved by this PR if the closing keyword did not activate.
  • Verified that all checks pass on the parent branch after merge.

@rksharma-owg
rksharma-owg force-pushed the codex/2112-preserve-aggregate-exceptions branch from 93a2f47 to 18f362c Compare August 30, 2026 15:52
@rksharma-owg

Copy link
Copy Markdown
Author

Refreshed this branch onto current main at 18f362c7; it remains conflict-free and git diff --check passes.

The three previous attached check suites were not code/test failures: they contained zero jobs and automatically expired after waiting 30 days for first-time-contributor workflow approval. The same contribution SHA had already passed all seven jobs in fork CI, including PSScriptAnalyzer and Windows PowerShell Pester.

Fresh upstream runs were created by this update and currently show action_required; a cisagov maintainer will need to approve them before they can execute.

@buidav buidav added the public-reported This issue is reported by the public users of the tool. label Sep 10, 2026
@rksharma-owg
rksharma-owg force-pushed the codex/2112-preserve-aggregate-exceptions branch from 18f362c to 3841939 Compare September 30, 2026 03:24

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

public-reported This issue is reported by the public users of the tool.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Connect-Tenant hides the underlying authentication exception in some scenarios

2 participants