Skip to content

Improve CLI error reporting on abp.io auth/license failure#25443

Merged
enisn merged 5 commits into
rel-10.4from
maliming/fix-25403-cli-auth-error
May 16, 2026
Merged

Improve CLI error reporting on abp.io auth/license failure#25443
enisn merged 5 commits into
rel-10.4from
maliming/fix-25403-cli-auth-error

Conversation

@maliming
Copy link
Copy Markdown
Member

@maliming maliming commented May 15, 2026

Fixes #25403.

When abp.io rejects a CLI download (HTTP 403 + HTML error page), the CLI used to surface '<' is an invalid start of a value with no hint about license/auth.

  • RemoteServiceExceptionHandler.GetAbpRemoteServiceErrorAsync now catches both System.Text.Json.JsonException and Newtonsoft.Json.JsonException (was only Newtonsoft.Json.JsonReaderException, which the active AbpSystemTextJsonSerializer never throws).
  • abp.io 401/403 responses are wrapped in CliUsageException with an abp login / license hint. User-supplied --template-source URLs use the generic handler.

Added unit tests for RemoteServiceExceptionHandler.

- RemoteServiceExceptionHandler: catch all non-cancellation exceptions
  during error response deserialization (was only catching Newtonsoft
  JsonReaderException, but the active serializer throws System.Text.Json
  JsonException, leaking '<' is an invalid start of a value to users)
- AbpIoSourceCodeStore: wrap 401/403 responses from abp.io endpoints in
  CliUsageException with login + license hint, so users get an actionable
  message instead of HTML-as-JSON parse failure
- Add unit tests for RemoteServiceExceptionHandler covering HTML body,
  valid JSON error, 5xx, and OperationCanceledException propagation
Copilot AI review requested due to automatic review settings May 15, 2026 08:58
@maliming maliming added this to the 10.4-patch milestone May 15, 2026
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 ABP CLI diagnostics when abp.io returns non-JSON error pages (notably 401/403 HTML responses during template/source downloads), so users get actionable guidance instead of JSON parse errors.

Changes:

  • Broadened RemoteServiceExceptionHandler JSON-deserialization error handling to tolerate System.Text.Json exceptions (and other non-cancellation failures) and fall back to generic HTTP status messaging.
  • Added abp.io–specific response handling in AbpIoSourceCodeStore to throw a CliUsageException with guidance on login/license checks for 401/403 responses.
  • Added unit tests covering HTML body handling, valid JSON error bodies, 5xx behavior, and OperationCanceledException propagation.

Reviewed changes

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

File Description
framework/test/Volo.Abp.Cli.Core.Tests/Volo/Abp/Cli/ProjectBuilding/RemoteServiceExceptionHandler_Tests.cs Adds unit tests validating improved error handling and cancellation propagation.
framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/ProjectBuilding/RemoteServiceExceptionHandler.cs Makes remote error parsing resilient to non-Newtonsoft JSON exceptions and non-cancellation failures.
framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/ProjectBuilding/AbpIoSourceCodeStore.cs Introduces a shared helper to convert 401/403 abp.io responses into actionable CLI usage errors.

- Drop manual HttpResponseMessage dispose in DownloadSourceCodeContentAsync,
  consistent with Volo.Abp.Http.Client.ClientProxyBase and ASP.NET Core OAuth
  handlers (default HttpCompletionOption.ResponseContentRead does not need
  manual disposal)
- Fix "occured" -> "occurred" typos in two log lines
- Remove unused using Microsoft.Extensions.Options from test
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

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

…r test

Asserting GetType() == typeof(Exception) locks the test to the base
type. The remaining message assertions already cover the intent of
ensuring the underlying JSON parse error does not surface.
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

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

@codecov
Copy link
Copy Markdown

codecov Bot commented May 15, 2026

Codecov Report

❌ Patch coverage is 12.50000% with 35 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.39%. Comparing base (dd0d9ad) to head (9ddc151).
⚠️ Report is 11 commits behind head on rel-10.4.

Files with missing lines Patch % Lines
...lo/Abp/Cli/ProjectBuilding/AbpIoSourceCodeStore.cs 0.00% 35 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           rel-10.4   #25443      +/-   ##
============================================
+ Coverage     49.30%   49.39%   +0.09%     
============================================
  Files          3668     3670       +2     
  Lines        123343   123594     +251     
  Branches       9424     9452      +28     
============================================
+ Hits          60816    61055     +239     
- Misses        60702    60705       +3     
- Partials       1825     1834       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 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.

- Narrow GetAbpRemoteServiceErrorAsync catch to JSON deserialization
  exceptions (System.Text.Json + Newtonsoft) so OOM and other runtime
  errors are no longer swallowed
- Dispose HttpResponseMessage in DownloadSourceCodeContentAsync via
  finally block, matching the pattern used by sibling methods
- Use generic EnsureSuccessfulHttpResponseAsync for user-supplied
  TemplateSource downloads so non-abp.io 401/403 responses don't show
  a misleading abp.io license hint
- Add tests for Newtonsoft JsonException handling and non-JSON exception
  propagation
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

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

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

- Use CliUrls.WwwAbpIo instead of hardcoded abp.io host/URL in the
  401/403 license hint, so dev/staging environments show the right URL
- Include server-provided RemoteServiceErrorResponse details (e.g. Code:
  LicenseExpired) in the CliUsageException message when available
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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@enisn enisn merged commit 521f6c9 into rel-10.4 May 16, 2026
5 of 6 checks passed
@enisn enisn deleted the maliming/fix-25403-cli-auth-error branch May 16, 2026 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants