Skip to content

Commit 764b4d7

Browse files
halspanghalspang
authored
Fix issue where state did not reset on actor error (#789)
When an actor call fails, it is imperative that the state is reset. In the remoting path, we were catching the exception and returning an error response instead of propagating the exception. Without this, the error path was never triggered. This allowed for state from failed requests to persist on subsequent requests. #762 Signed-off-by: Hal Spang <[email protected]>
1 parent d43de2b commit 764b4d7

File tree

19 files changed

+471
-59
lines changed

19 files changed

+471
-59
lines changed

‎all.sln

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Dapr.E2E.Test.App.Grpc", "t
8383
EndProject
8484
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ConfigurationApi", "examples\Client\ConfigurationApi\ConfigurationApi.csproj", "{F80F837E-D2FC-4FFC-B68F-3CF0EC015F66}"
8585
EndProject
86+
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Dapr.E2E.Test.App.ReentrantActors", "test\Dapr.E2E.Test.App.ReentrantActor\Dapr.E2E.Test.App.ReentrantActors.csproj", "{5BE7F505-7D77-4C3A-ABFD-54088774DAA7}"
87+
EndProject
8688
Global
8789
GlobalSection(SolutionConfigurationPlatforms) = preSolution
8890
Debug|Any CPU = Debug|Any CPU
@@ -205,6 +207,10 @@ Global
205207
{F80F837E-D2FC-4FFC-B68F-3CF0EC015F66}.Debug|Any CPU.Build.0 = Debug|Any CPU
206208
{F80F837E-D2FC-4FFC-B68F-3CF0EC015F66}.Release|Any CPU.ActiveCfg = Release|Any CPU
207209
{F80F837E-D2FC-4FFC-B68F-3CF0EC015F66}.Release|Any CPU.Build.0 = Release|Any CPU
210+
{5BE7F505-7D77-4C3A-ABFD-54088774DAA7}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
211+
{5BE7F505-7D77-4C3A-ABFD-54088774DAA7}.Debug|Any CPU.Build.0 = Debug|Any CPU
212+
{5BE7F505-7D77-4C3A-ABFD-54088774DAA7}.Release|Any CPU.ActiveCfg = Release|Any CPU
213+
{5BE7F505-7D77-4C3A-ABFD-54088774DAA7}.Release|Any CPU.Build.0 = Release|Any CPU
208214
EndGlobalSection
209215
GlobalSection(SolutionProperties) = preSolution
210216
HideSolutionNode = FALSE
@@ -242,6 +248,7 @@ Global
242248
{2AED1542-A8ED-488D-B6D0-E16AB5D6EF6C} = {DD020B34-460F-455F-8D17-CF4A949F100B}
243249
{E8212911-344B-4638-ADC3-B215BCDCAFD1} = {DD020B34-460F-455F-8D17-CF4A949F100B}
244250
{F80F837E-D2FC-4FFC-B68F-3CF0EC015F66} = {A7F41094-8648-446B-AECD-DCC2CC871F73}
251+
{5BE7F505-7D77-4C3A-ABFD-54088774DAA7} = {DD020B34-460F-455F-8D17-CF4A949F100B}
245252
EndGlobalSection
246253
GlobalSection(ExtensibilityGlobals) = postSolution
247254
SolutionGuid = {65220BF2-EAE1-4CB2-AA58-EBE80768CB40}

‎src/Dapr.Actors.AspNetCore/ActorsEndpointRouteBuilderExtensions.cs

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,17 @@
1111
// limitations under the License.
1212
// ------------------------------------------------------------------------
1313

14+
using System;
15+
using System.Text;
16+
using Dapr.Actors;
17+
using Dapr.Actors.Communication;
18+
using Dapr.Actors.Runtime;
19+
using Microsoft.AspNetCore.Http;
20+
using Microsoft.AspNetCore.Routing;
21+
using Microsoft.Extensions.DependencyInjection;
22+
1423
namespace Microsoft.AspNetCore.Builder
1524
{
16-
using System;
17-
using Dapr.Actors;
18-
using Dapr.Actors.Runtime;
19-
using Microsoft.AspNetCore.Http;
20-
using Microsoft.AspNetCore.Routing;
21-
using Microsoft.Extensions.DependencyInjection;
22-
2325
/// <summary>
2426
/// Contains extension methods for using Dapr Actors with endpoint routing.
2527
/// </summary>
@@ -111,6 +113,13 @@ private static IEndpointConventionBuilder MapActorMethodEndpoint(this IEndpointR
111113

112114
await context.Response.Body.WriteAsync(body, 0, body.Length); // add response message body
113115
}
116+
catch (Exception ex)
117+
{
118+
var (header, body) = CreateExceptionResponseMessage(ex);
119+
120+
context.Response.Headers.Add(Constants.ErrorResponseHeaderName, header);
121+
await context.Response.Body.WriteAsync(body, 0, body.Length);
122+
}
114123
finally
115124
{
116125
ActorReentrancyContextAccessor.ReentrancyContext = null;
@@ -160,7 +169,6 @@ private static IEndpointConventionBuilder MapTimerEndpoint(this IEndpointRouteBu
160169
}).WithDisplayName(b => "Dapr Actors Timer");
161170
}
162171

163-
164172
private static IEndpointConventionBuilder MapActorHealthChecks(this IEndpointRouteBuilder endpoints)
165173
{
166174
var builder = endpoints.MapHealthChecks("/healthz");
@@ -177,7 +185,20 @@ private static IEndpointConventionBuilder MapActorHealthChecks(this IEndpointRou
177185
return builder.WithDisplayName(b => "Dapr Actors Health Check");
178186
}
179187

180-
private class CompositeEndpointConventionBuilder : IEndpointConventionBuilder
188+
private static Tuple<string, byte[]> CreateExceptionResponseMessage(Exception ex)
189+
{
190+
var responseHeader = new ActorResponseMessageHeader();
191+
responseHeader.AddHeader("HasRemoteException", Array.Empty<byte>());
192+
var headerSerializer = new ActorMessageHeaderSerializer();
193+
var responseHeaderBytes = headerSerializer.SerializeResponseHeader(responseHeader);
194+
var serializedHeader = Encoding.UTF8.GetString(responseHeaderBytes, 0, responseHeaderBytes.Length);
195+
196+
var responseMsgBody = ActorInvokeException.FromException(ex);
197+
198+
return new Tuple<string, byte[]>(serializedHeader, responseMsgBody);
199+
}
200+
201+
private class CompositeEndpointConventionBuilder : IEndpointConventionBuilder
181202
{
182203
private readonly IEndpointConventionBuilder[] inner;
183204

‎src/Dapr.Actors/Runtime/ActorManager.cs

Lines changed: 8 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -114,22 +114,14 @@ async Task<Tuple<string, byte[]>> RequestFunc(Actor actor, CancellationToken ct)
114114
{
115115
IActorResponseMessageBody responseMsgBody = null;
116116

117-
try
118-
{
119-
responseMsgBody = (IActorResponseMessageBody)await methodDispatcher.DispatchAsync(
120-
actor,
121-
actorMessageHeader.MethodId,
122-
actorMessageBody,
123-
this.messageBodyFactory,
124-
ct);
125-
126-
return this.CreateResponseMessage(responseMsgBody, interfaceId);
127-
}
128-
catch (Exception exception)
129-
{
130-
// return exception response message
131-
return this.CreateExceptionResponseMessage(exception);
132-
}
117+
responseMsgBody = (IActorResponseMessageBody)await methodDispatcher.DispatchAsync(
118+
actor,
119+
actorMessageHeader.MethodId,
120+
actorMessageBody,
121+
this.messageBodyFactory,
122+
ct);
123+
124+
return this.CreateResponseMessage(responseMsgBody, interfaceId);
133125
}
134126

135127
return await this.DispatchInternalAsync(actorId, actorMethodContext, RequestFunc, cancellationToken);
@@ -391,17 +383,5 @@ private Tuple<string, byte[]> CreateResponseMessage(IActorResponseMessageBody ms
391383

392384
return new Tuple<string, byte[]>(string.Empty, responseMsgBodyBytes);
393385
}
394-
395-
private Tuple<string, byte[]> CreateExceptionResponseMessage(Exception ex)
396-
{
397-
var responseHeader = new ActorResponseMessageHeader();
398-
responseHeader.AddHeader("HasRemoteException", Array.Empty<byte>());
399-
var responseHeaderBytes = this.serializersManager.GetHeaderSerializer().SerializeResponseHeader(responseHeader);
400-
var serializedHeader = Encoding.UTF8.GetString(responseHeaderBytes, 0, responseHeaderBytes.Length);
401-
402-
var responseMsgBody = ActorInvokeException.FromException(ex);
403-
404-
return new Tuple<string, byte[]>(serializedHeader, responseMsgBody);
405-
}
406386
}
407387
}

‎test/Dapr.E2E.Test.Actors/RegressionActor/IRegression762Actor.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// ------------------------------------------------------------------------
2+
// Copyright 2021 The Dapr Authors
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
// http://www.apache.org/licenses/LICENSE-2.0
7+
// Unless required by applicable law or agreed to in writing, software
8+
// distributed under the License is distributed on an "AS IS" BASIS,
9+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
10+
// See the License for the specific language governing permissions and
11+
// limitations under the License.
12+
// ------------------------------------------------------------------------
13+
14+
using System.Threading.Tasks;
15+
using Dapr.Actors;
16+
17+
/// <summary>
18+
/// This actor is being used to test the specific regression found in:
19+
///
20+
/// https://github.com/dapr/dotnet-sdk/issues/762
21+
/// </summary>
22+
namespace Dapr.E2E.Test.Actors.ErrorTesting
23+
{
24+
public interface IRegression762Actor : IPingActor, IActor
25+
{
26+
Task<string> GetState(string id);
27+
28+
Task SaveState(StateCall call);
29+
30+
Task RemoveState(string id);
31+
}
32+
}

‎test/Dapr.E2E.Test.Actors/RegressionActor/StateCall.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// ------------------------------------------------------------------------
2+
// Copyright 2021 The Dapr Authors
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
// http://www.apache.org/licenses/LICENSE-2.0
7+
// Unless required by applicable law or agreed to in writing, software
8+
// distributed under the License is distributed on an "AS IS" BASIS,
9+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
10+
// See the License for the specific language governing permissions and
11+
// limitations under the License.
12+
// ------------------------------------------------------------------------
13+
14+
namespace Dapr.E2E.Test.Actors.ErrorTesting
15+
{
16+
public class StateCall
17+
{
18+
public string Key { get; set; }
19+
public string Value { get; set; }
20+
public string Operation { get; set; }
21+
}
22+
}

‎test/Dapr.E2E.Test.App/Actors/ReentrantActor.cs renamed to ‎test/Dapr.E2E.Test.App.ReentrantActor/Actors/ReentrantActor.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
// limitations under the License.
1212
// ------------------------------------------------------------------------
1313

14-
using System.Collections.Generic;
1514
using System.Threading.Tasks;
1615
using Dapr.Actors.Runtime;
1716

‎test/Dapr.E2E.Test.App.ReentrantActor/Dapr.E2E.Test.App.ReentrantActors.csproj

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<Project Sdk="Microsoft.NET.Sdk.Web">
2+
3+
<PropertyGroup>
4+
<TargetFrameworks>netcoreapp3.1;net5</TargetFrameworks>
5+
</PropertyGroup>
6+
7+
<PropertyGroup Condition=" '$(RunConfiguration)' == 'Dapr.E2E.Test.App.ReentrantActor' " />
8+
<ItemGroup>
9+
<ProjectReference Include="..\..\src\Dapr.AspNetCore\Dapr.AspNetCore.csproj" />
10+
<ProjectReference Include="..\..\src\Dapr.Actors.AspNetCore\Dapr.Actors.AspNetCore.csproj" />
11+
<ProjectReference Include="..\Dapr.E2E.Test.Actors\Dapr.E2E.Test.Actors.csproj" />
12+
</ItemGroup>
13+
14+
</Project>

‎test/Dapr.E2E.Test.App.ReentrantActor/Program.cs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// ------------------------------------------------------------------------
2+
// Copyright 2021 The Dapr Authors
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
// http://www.apache.org/licenses/LICENSE-2.0
7+
// Unless required by applicable law or agreed to in writing, software
8+
// distributed under the License is distributed on an "AS IS" BASIS,
9+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
10+
// See the License for the specific language governing permissions and
11+
// limitations under the License.
12+
// ------------------------------------------------------------------------
13+
14+
using Microsoft.AspNetCore.Hosting;
15+
using Microsoft.Extensions.Hosting;
16+
17+
namespace Dapr.E2E.Test.App.ReentrantActors
18+
{
19+
public class Program
20+
{
21+
public static void Main(string[] args)
22+
{
23+
CreateHostBuilder(args).Build().Run();
24+
}
25+
26+
public static IHostBuilder CreateHostBuilder(string[] args) =>
27+
Host.CreateDefaultBuilder(args)
28+
.ConfigureWebHostDefaults(webBuilder =>
29+
{
30+
webBuilder.UseStartup<Startup>();
31+
});
32+
}
33+
}

‎test/Dapr.E2E.Test.App.ReentrantActor/Startup.cs

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// ------------------------------------------------------------------------
2+
// Copyright 2021 The Dapr Authors
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
// http://www.apache.org/licenses/LICENSE-2.0
7+
// Unless required by applicable law or agreed to in writing, software
8+
// distributed under the License is distributed on an "AS IS" BASIS,
9+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
10+
// See the License for the specific language governing permissions and
11+
// limitations under the License.
12+
// ------------------------------------------------------------------------
13+
14+
using Dapr.E2E.Test.Actors.Reentrancy;
15+
using Microsoft.AspNetCore.Builder;
16+
using Microsoft.AspNetCore.Hosting;
17+
using Microsoft.Extensions.DependencyInjection;
18+
using Microsoft.Extensions.Hosting;
19+
20+
namespace Dapr.E2E.Test.App.ReentrantActors
21+
{
22+
public class Startup
23+
{
24+
// This method gets called by the runtime. Use this method to add services to the container.
25+
// For more information on how to configure your application, visit https://go.microsoft.com/fwlink/?LinkID=398940
26+
public void ConfigureServices(IServiceCollection services)
27+
{
28+
services.AddActors(options =>
29+
{
30+
options.Actors.RegisterActor<ReentrantActor>();
31+
options.ReentrancyConfig = new() { Enabled = true };
32+
});
33+
}
34+
35+
// This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
36+
public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
37+
{
38+
if (env.IsDevelopment())
39+
{
40+
app.UseDeveloperExceptionPage();
41+
}
42+
43+
app.UseRouting();
44+
45+
app.UseEndpoints(endpoints =>
46+
{
47+
endpoints.MapActorsHandlers();
48+
});
49+
}
50+
}
51+
}

‎test/Dapr.E2E.Test.App.ReentrantActor/appsettings.Development.json

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"Logging": {
3+
"LogLevel": {
4+
"Default": "Information",
5+
"Microsoft": "Warning",
6+
"Microsoft.Hosting.Lifetime": "Information"
7+
}
8+
}
9+
}

0 commit comments

Comments
 (0)