Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only set specific type of exception's data to errorInfo.Data. #20812

Merged
merged 2 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions framework/Volo.Abp.sln
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Volo.Abp.BlobStoring.Google
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Volo.Abp.BlobStoring.Google.Tests", "test\Volo.Abp.BlobStoring.Google.Tests\Volo.Abp.BlobStoring.Google.Tests.csproj", "{40FB8907-9CF7-44D0-8B5F-538AC6DAF8B9}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Volo.Abp.ExceptionHandling.Tests", "test\Volo.Abp.ExceptionHandling.Tests\Volo.Abp.ExceptionHandling.Tests.csproj", "{E50739A7-5E2F-4EB5-AEA9-554115CB9613}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -1405,6 +1407,10 @@ Global
{40FB8907-9CF7-44D0-8B5F-538AC6DAF8B9}.Debug|Any CPU.Build.0 = Debug|Any CPU
{40FB8907-9CF7-44D0-8B5F-538AC6DAF8B9}.Release|Any CPU.ActiveCfg = Release|Any CPU
{40FB8907-9CF7-44D0-8B5F-538AC6DAF8B9}.Release|Any CPU.Build.0 = Release|Any CPU
{E50739A7-5E2F-4EB5-AEA9-554115CB9613}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{E50739A7-5E2F-4EB5-AEA9-554115CB9613}.Debug|Any CPU.Build.0 = Debug|Any CPU
{E50739A7-5E2F-4EB5-AEA9-554115CB9613}.Release|Any CPU.ActiveCfg = Release|Any CPU
{E50739A7-5E2F-4EB5-AEA9-554115CB9613}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down Expand Up @@ -1642,6 +1648,7 @@ Global
{E1051CD0-9262-4869-832D-B951723F4DDE} = {5DF0E140-0513-4D0D-BE2E-3D4D85CD70E6}
{DEEB5200-BBF9-464D-9B7E-8FC035A27E94} = {5DF0E140-0513-4D0D-BE2E-3D4D85CD70E6}
{40FB8907-9CF7-44D0-8B5F-538AC6DAF8B9} = {447C8A77-E5F0-4538-8687-7383196D04EA}
{E50739A7-5E2F-4EB5-AEA9-554115CB9613} = {447C8A77-E5F0-4538-8687-7383196D04EA}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {BB97ECF4-9A84-433F-A80B-2A3285BDD1D5}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
namespace Volo.Abp.AspNetCore.ExceptionHandling;
using System;
using System.Collections.Generic;

namespace Volo.Abp.AspNetCore.ExceptionHandling;

public class AbpExceptionHandlingOptions
{
public bool SendExceptionsDetailsToClients { get; set; } = false;

public bool SendStackTraceToClients { get; set; } = true;

public List<Type> SendExceptionDataToClientTypes { get; } = new List<Type>();
}
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,10 @@ protected virtual RemoteServiceErrorInfo CreateErrorInfoWithoutCode(Exception ex
errorInfo.Message = L["InternalServerErrorMessage"];
}

errorInfo.Data = exception.Data;
if (options.SendExceptionDataToClientTypes.Any(t => t.IsInstanceOfType(exception)))
{
errorInfo.Data = exception.Data;
}

return errorInfo;
}
Expand Down Expand Up @@ -326,7 +329,11 @@ protected virtual AbpExceptionHandlingOptions CreateDefaultOptions()
return new AbpExceptionHandlingOptions
{
SendExceptionsDetailsToClients = false,
SendStackTraceToClients = true
SendStackTraceToClients = true,
SendExceptionDataToClientTypes =
{
typeof(IBusinessException)
}
Copy link
Member

Choose a reason for hiding this comment

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

We should also pass the SendExceptionDataToClientTypes parameter to the Convert method calls. Otherwise, we can't configure the default option as intended, since CreateDefaultOptions method only adds the IBusinessException interface.

Current services that use this option: AbpExceptionHandlingMiddleware, UserExceptionInformer, AbpExceptionFilter, AbpExceptionPageFilter, and ErrorController.

Copy link
Member Author

Choose a reason for hiding this comment

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

};
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<Project Sdk="Microsoft.NET.Sdk">

<Import Project="..\..\..\common.test.props" />

<PropertyGroup>
<TargetFramework>net8.0</TargetFramework>
<RootNamespace />
</PropertyGroup>

<ItemGroup>
<ProjectReference Include="..\..\src\Volo.Abp.Autofac\Volo.Abp.Autofac.csproj" />
<ProjectReference Include="..\..\src\Volo.Abp.ExceptionHandling\Volo.Abp.ExceptionHandling.csproj" />
<ProjectReference Include="..\AbpTestBase\AbpTestBase.csproj" />
<PackageReference Include="Microsoft.NET.Test.Sdk" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
using Volo.Abp.Testing;

namespace Volo.Abp.ExceptionHandling;

public class AbpExceptionHandlingTestBase : AbpIntegratedTest<AbpExceptionHandlingTestModule>
{
protected override void SetAbpApplicationCreationOptions(AbpApplicationCreationOptions options)
{
options.UseAutofac();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
using Volo.Abp.Autofac;
using Volo.Abp.Modularity;

namespace Volo.Abp.ExceptionHandling;

[DependsOn(
typeof(AbpExceptionHandlingModule),
typeof(AbpTestBaseModule),
typeof(AbpAutofacModule)
)]
public class AbpExceptionHandlingTestModule : AbpModule
{

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
using Shouldly;
using Volo.Abp.AspNetCore.ExceptionHandling;
using Xunit;

namespace Volo.Abp.ExceptionHandling;

public class ExceptionToErrorInfoConverter_Tests : AbpExceptionHandlingTestBase
{
private readonly IExceptionToErrorInfoConverter _exceptionToErrorInfoConverter;

public ExceptionToErrorInfoConverter_Tests()
{
_exceptionToErrorInfoConverter = GetRequiredService<IExceptionToErrorInfoConverter>();
}

[Fact]
public void SendExceptionDataToClientTypes_Test()
{
var exception = new AbpException("test message")
{
Data =
{
["my_data"] = "my_data_value",
["my_data2"] = 42
}
};
var errorInfo = _exceptionToErrorInfoConverter.Convert(exception);
errorInfo.Data.ShouldBeNull();

errorInfo = _exceptionToErrorInfoConverter.Convert(exception, options =>
{
options.SendExceptionDataToClientTypes.Add(typeof(AbpException));
});
errorInfo.Data.ShouldNotBeNull();
errorInfo.Data.Keys.Count.ShouldBe(2);
errorInfo.Data["my_data"].ShouldBe("my_data_value");
errorInfo.Data["my_data2"].ShouldBe(42);

var businessException = new BusinessException("test message")
{
Data =
{
["my_data"] = "my_data_value",
["my_data2"] = 42
}
};
errorInfo = _exceptionToErrorInfoConverter.Convert(businessException);
errorInfo.Data.ShouldNotBeNull();
errorInfo.Data.Keys.Count.ShouldBe(2);
errorInfo.Data["my_data"].ShouldBe("my_data_value");
errorInfo.Data["my_data2"].ShouldBe(42);
}
}
Loading