Skip to content

fix(cli): surface real cause when discoverAzureBot fails#2833

Merged
heyitsaamir merged 1 commit into
mainfrom
fix/2830-discover-azure-bot-error-handling
May 14, 2026
Merged

fix(cli): surface real cause when discoverAzureBot fails#2833
heyitsaamir merged 1 commit into
mainfrom
fix/2830-discover-azure-bot-error-handling

Conversation

@singhk97
Copy link
Copy Markdown
Collaborator

@singhk97 singhk97 commented May 12, 2026

Fixes #2830.

discoverAzureBot and its helpers collapsed every az failure into null, indistinguishable from a real "bot not found." app update --json then returned a misleading NOT_FOUND_AZURE_BOT; under --yes, an agent could try to recreate a bot that already exists but couldn't be read.

Rebased on top of #2825's fallback chain. Only the bug-shaped catches are removed; the two structural ones are kept:

  • findBotByName outer catch — removed (pure suppression).
  • findBotByMsaAppIdViaList outer catch — removed (pure suppression).
  • findBotByMsaAppIdViaList per-bot catch — kept (per-element resilience inside Promise.all).
  • findBotByMsaAppIdViaGraph outer catch — kept (intentional fallback signal: routes "graph extension unavailable" to the list path; pinned by fix(cli): doctor finds Azure bot when resource name != botId #2825's tests).

discoverAzureBot rewraps the surviving catch as CliError('API_ARM_ERROR', 'Failed to discover Azure bot: <stderr>') — preferring error.stderr over error.message so the real az diagnostic surfaces instead of Command failed: cmd /c az.cmd .... app doctor wraps the call so a throw becomes a fail entry instead of terminating the report.

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings May 12, 2026 23:23
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Improves Azure bot discovery error reporting in the Teams CLI so discoverAzureBot() only returns null for the true “not found” case, and otherwise surfaces actionable Azure CLI failures (auth/network/permissions/etc.) to callers (esp. --json automation), while keeping app doctor resilient.

Changes:

  • Update discoverAzureBot() to rethrow non-empty-results failures as CliError('API_ARM_ERROR', ...) and prefer stderr for the message.
  • Make app doctor record a single failing check when discovery throws, rather than continuing with misleading “not found” output.
  • Add unit tests covering null vs thrown-error behavior and message preservation.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
packages/cli/src/apps/bot-handler.ts Stop swallowing az failures; throw CliError('API_ARM_ERROR') with stderr-first diagnostic text.
packages/cli/src/commands/app/doctor.ts Catch discovery errors and report them as a dedicated fail entry; avoid “Azure bot not found” when discovery failed.
packages/cli/tests/discover-azure-bot.test.ts Add tests validating null-on-empty-results and thrown-error behavior for discoverAzureBot().

Comment thread packages/cli/tests/discover-azure-bot.test.ts Outdated
Comment thread packages/cli/src/commands/app/doctor.ts
@singhk97
Copy link
Copy Markdown
Collaborator Author

I simulated a network error by overriding the windows DNS lookup for management.azure.com, and have it resolve to 127.0.0.1 (my local machine). Since there's no listener on that port a network calls to the endpoint will result in no connection could be made because the target machine actively refused it error.

Without the fix:

image

With the fix:

image

discoverAzureBot's helper catches and outer catch collapsed every az
failure into null, indistinguishable from a legitimate "bot not found"
result and causing --yes/--json callers to report a misleading
NOT_FOUND_AZURE_BOT code.

Removes the bug-shaped catches in findBotByName and the outer catch of
findBotByMsaAppIdViaList so az errors propagate. Keeps the
findBotByMsaAppIdViaGraph catch (intentional fallback to list when the
resource-graph extension is unavailable) and the per-bot inner catch
inside the list scan (per-element resilience). The outer catch in
discoverAzureBot now rewraps as CliError('API_ARM_ERROR') with the
original stderr/message preserved. Doctor wraps the call so a thrown
error becomes one fail entry instead of terminating the report.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@singhk97 singhk97 force-pushed the fix/2830-discover-azure-bot-error-handling branch from c2e32d0 to 4e92688 Compare May 13, 2026 20:34
@heyitsaamir heyitsaamir merged commit 53a4dda into main May 14, 2026
3 checks passed
@heyitsaamir heyitsaamir deleted the fix/2830-discover-azure-bot-error-handling branch May 14, 2026 03:53
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.

[Bug]: discoverAzureBot swallows all errors as "not found", causing misleading messages and risk of duplicate bot creation

3 participants