Browse Source

Address Copilot review feedback (round 2)

- 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
pull/25443/head
maliming 3 months ago
parent
commit
9ddc15114f
No known key found for this signature in database GPG Key ID: A646B9CB645ECEA4
  1. 20
      framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/ProjectBuilding/AbpIoSourceCodeStore.cs
  2. 9
      framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/ProjectBuilding/RemoteServiceExceptionHandler.cs
  3. 60
      framework/test/Volo.Abp.Cli.Core.Tests/Volo/Abp/Cli/ProjectBuilding/RemoteServiceExceptionHandler_Tests.cs

20
framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/ProjectBuilding/AbpIoSourceCodeStore.cs

@ -329,6 +329,8 @@ public class AbpIoSourceCodeStore : ISourceCodeStore, ITransientDependency
private async Task<byte[]> DownloadSourceCodeContentAsync(SourceCodeDownloadInputDto input)
{
var url = $"{CliUrls.WwwAbpIo}api/download/{input.Type}/";
var isAbpIoDownload = input.TemplateSource.IsNullOrWhiteSpace();
var downloadUrl = isAbpIoDownload ? url : input.TemplateSource;
HttpResponseMessage responseMessage = null;
@ -336,7 +338,7 @@ public class AbpIoSourceCodeStore : ISourceCodeStore, ITransientDependency
{
var client = _cliHttpClientFactory.CreateClient(timeout: TimeSpan.FromMinutes(5));
if (input.TemplateSource.IsNullOrWhiteSpace())
if (isAbpIoDownload)
{
responseMessage = await client.PostAsync(
url,
@ -350,7 +352,15 @@ public class AbpIoSourceCodeStore : ISourceCodeStore, ITransientDependency
_cliHttpClientFactory.GetCancellationToken());
}
await EnsureAbpIoSuccessfulResponseAsync(responseMessage);
if (isAbpIoDownload)
{
await EnsureAbpIoSuccessfulResponseAsync(responseMessage);
}
else
{
await RemoteServiceExceptionHandler.EnsureSuccessfulHttpResponseAsync(responseMessage);
}
return await responseMessage.Content.ReadAsByteArrayAsync();
}
catch (Exception ex)
@ -366,10 +376,14 @@ public class AbpIoSourceCodeStore : ISourceCodeStore, ITransientDependency
throw;
}
Console.WriteLine("Error occurred while downloading source-code from {0} : {1}{2}{3}", url,
Console.WriteLine("Error occurred while downloading source-code from {0} : {1}{2}{3}", downloadUrl,
responseMessage?.ToString(), Environment.NewLine, ex.Message);
throw;
}
finally
{
responseMessage?.Dispose();
}
}
private static bool IsNetworkSource(string source)

9
framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/ProjectBuilding/RemoteServiceExceptionHandler.cs

@ -7,6 +7,8 @@ using System.Threading.Tasks;
using Volo.Abp.DependencyInjection;
using Volo.Abp.Http;
using Volo.Abp.Json;
using NewtonsoftJsonException = Newtonsoft.Json.JsonException;
using SystemJsonException = System.Text.Json.JsonException;
namespace Volo.Abp.Cli.ProjectBuilding;
@ -52,7 +54,7 @@ public class RemoteServiceExceptionHandler : IRemoteServiceExceptionHandler, ITr
await responseMessage.Content.ReadAsStringAsync()
);
}
catch (Exception ex) when (ex is not OperationCanceledException)
catch (Exception ex) when (IsJsonException(ex))
{
return null;
}
@ -96,4 +98,9 @@ public class RemoteServiceExceptionHandler : IRemoteServiceExceptionHandler, ITr
return sbError.ToString();
}
private static bool IsJsonException(Exception ex)
{
return ex is SystemJsonException or NewtonsoftJsonException;
}
}

60
framework/test/Volo.Abp.Cli.Core.Tests/Volo/Abp/Cli/ProjectBuilding/RemoteServiceExceptionHandler_Tests.cs

@ -4,6 +4,7 @@ using System.Net.Http;
using System.Threading.Tasks;
using Shouldly;
using Volo.Abp.Cli.ProjectBuilding;
using Volo.Abp.Json;
using Volo.Abp.Json.SystemTextJson;
using Xunit;
@ -117,6 +118,40 @@ public class RemoteServiceExceptionHandler_Tests
result.ShouldBeNull();
}
[Fact]
public async Task GetAbpRemoteServiceErrorAsync_Should_Return_Null_For_Newtonsoft_JsonException()
{
var handler = new RemoteServiceExceptionHandler(
new ThrowingJsonSerializer(new Newtonsoft.Json.JsonException("Invalid JSON"))
);
var response = new HttpResponseMessage(HttpStatusCode.Forbidden)
{
Content = new StringContent("{}")
};
var result = await handler.GetAbpRemoteServiceErrorAsync(response);
result.ShouldBeNull();
}
[Fact]
public async Task GetAbpRemoteServiceErrorAsync_Should_Propagate_Non_Json_Exceptions()
{
var handler = new RemoteServiceExceptionHandler(
new ThrowingJsonSerializer(new InvalidOperationException("Unexpected serializer failure"))
);
var response = new HttpResponseMessage(HttpStatusCode.Forbidden)
{
Content = new StringContent("{}")
};
var exception = await Should.ThrowAsync<InvalidOperationException>(
() => handler.GetAbpRemoteServiceErrorAsync(response)
);
exception.Message.ShouldBe("Unexpected serializer failure");
}
private class CanceledStringContent : HttpContent
{
protected override Task SerializeToStreamAsync(System.IO.Stream stream, System.Net.TransportContext context)
@ -130,4 +165,29 @@ public class RemoteServiceExceptionHandler_Tests
return false;
}
}
private class ThrowingJsonSerializer : IJsonSerializer
{
private readonly Exception _exception;
public ThrowingJsonSerializer(Exception exception)
{
_exception = exception;
}
public string Serialize(object obj, bool camelCase = true, bool indented = false)
{
throw new NotImplementedException();
}
public T Deserialize<T>(string jsonString, bool camelCase = true)
{
throw _exception;
}
public object Deserialize(Type type, string jsonString, bool camelCase = true)
{
throw _exception;
}
}
}

Loading…
Cancel
Save