Skip to content

Commit

Permalink
Fix issue where state did not reset on actor error (#789)
Browse files Browse the repository at this point in the history
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 <halspang@microsoft.com>
  • Loading branch information
halspang authored Jan 13, 2022
1 parent d43de2b commit 764b4d7
Show file tree
Hide file tree
Showing 19 changed files with 471 additions and 59 deletions.
7 changes: 7 additions & 0 deletions all.sln
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Dapr.E2E.Test.App.Grpc", "t
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ConfigurationApi", "examples\Client\ConfigurationApi\ConfigurationApi.csproj", "{F80F837E-D2FC-4FFC-B68F-3CF0EC015F66}"
EndProject
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}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -205,6 +207,10 @@ Global
{F80F837E-D2FC-4FFC-B68F-3CF0EC015F66}.Debug|Any CPU.Build.0 = Debug|Any CPU
{F80F837E-D2FC-4FFC-B68F-3CF0EC015F66}.Release|Any CPU.ActiveCfg = Release|Any CPU
{F80F837E-D2FC-4FFC-B68F-3CF0EC015F66}.Release|Any CPU.Build.0 = Release|Any CPU
{5BE7F505-7D77-4C3A-ABFD-54088774DAA7}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{5BE7F505-7D77-4C3A-ABFD-54088774DAA7}.Debug|Any CPU.Build.0 = Debug|Any CPU
{5BE7F505-7D77-4C3A-ABFD-54088774DAA7}.Release|Any CPU.ActiveCfg = Release|Any CPU
{5BE7F505-7D77-4C3A-ABFD-54088774DAA7}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down Expand Up @@ -242,6 +248,7 @@ Global
{2AED1542-A8ED-488D-B6D0-E16AB5D6EF6C} = {DD020B34-460F-455F-8D17-CF4A949F100B}
{E8212911-344B-4638-ADC3-B215BCDCAFD1} = {DD020B34-460F-455F-8D17-CF4A949F100B}
{F80F837E-D2FC-4FFC-B68F-3CF0EC015F66} = {A7F41094-8648-446B-AECD-DCC2CC871F73}
{5BE7F505-7D77-4C3A-ABFD-54088774DAA7} = {DD020B34-460F-455F-8D17-CF4A949F100B}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {65220BF2-EAE1-4CB2-AA58-EBE80768CB40}
Expand Down
39 changes: 30 additions & 9 deletions src/Dapr.Actors.AspNetCore/ActorsEndpointRouteBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,17 @@
// limitations under the License.
// ------------------------------------------------------------------------

using System;
using System.Text;
using Dapr.Actors;
using Dapr.Actors.Communication;
using Dapr.Actors.Runtime;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.DependencyInjection;

namespace Microsoft.AspNetCore.Builder
{
using System;
using Dapr.Actors;
using Dapr.Actors.Runtime;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.DependencyInjection;

/// <summary>
/// Contains extension methods for using Dapr Actors with endpoint routing.
/// </summary>
Expand Down Expand Up @@ -111,6 +113,13 @@ private static IEndpointConventionBuilder MapActorMethodEndpoint(this IEndpointR
await context.Response.Body.WriteAsync(body, 0, body.Length); // add response message body
}
catch (Exception ex)
{
var (header, body) = CreateExceptionResponseMessage(ex);
context.Response.Headers.Add(Constants.ErrorResponseHeaderName, header);
await context.Response.Body.WriteAsync(body, 0, body.Length);
}
finally
{
ActorReentrancyContextAccessor.ReentrancyContext = null;
Expand Down Expand Up @@ -160,7 +169,6 @@ private static IEndpointConventionBuilder MapTimerEndpoint(this IEndpointRouteBu
}).WithDisplayName(b => "Dapr Actors Timer");
}


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

private class CompositeEndpointConventionBuilder : IEndpointConventionBuilder
private static Tuple<string, byte[]> CreateExceptionResponseMessage(Exception ex)
{
var responseHeader = new ActorResponseMessageHeader();
responseHeader.AddHeader("HasRemoteException", Array.Empty<byte>());
var headerSerializer = new ActorMessageHeaderSerializer();
var responseHeaderBytes = headerSerializer.SerializeResponseHeader(responseHeader);
var serializedHeader = Encoding.UTF8.GetString(responseHeaderBytes, 0, responseHeaderBytes.Length);

var responseMsgBody = ActorInvokeException.FromException(ex);

return new Tuple<string, byte[]>(serializedHeader, responseMsgBody);
}

private class CompositeEndpointConventionBuilder : IEndpointConventionBuilder
{
private readonly IEndpointConventionBuilder[] inner;

Expand Down
36 changes: 8 additions & 28 deletions src/Dapr.Actors/Runtime/ActorManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -114,22 +114,14 @@ async Task<Tuple<string, byte[]>> RequestFunc(Actor actor, CancellationToken ct)
{
IActorResponseMessageBody responseMsgBody = null;

try
{
responseMsgBody = (IActorResponseMessageBody)await methodDispatcher.DispatchAsync(
actor,
actorMessageHeader.MethodId,
actorMessageBody,
this.messageBodyFactory,
ct);

return this.CreateResponseMessage(responseMsgBody, interfaceId);
}
catch (Exception exception)
{
// return exception response message
return this.CreateExceptionResponseMessage(exception);
}
responseMsgBody = (IActorResponseMessageBody)await methodDispatcher.DispatchAsync(
actor,
actorMessageHeader.MethodId,
actorMessageBody,
this.messageBodyFactory,
ct);

return this.CreateResponseMessage(responseMsgBody, interfaceId);
}

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

return new Tuple<string, byte[]>(string.Empty, responseMsgBodyBytes);
}

private Tuple<string, byte[]> CreateExceptionResponseMessage(Exception ex)
{
var responseHeader = new ActorResponseMessageHeader();
responseHeader.AddHeader("HasRemoteException", Array.Empty<byte>());
var responseHeaderBytes = this.serializersManager.GetHeaderSerializer().SerializeResponseHeader(responseHeader);
var serializedHeader = Encoding.UTF8.GetString(responseHeaderBytes, 0, responseHeaderBytes.Length);

var responseMsgBody = ActorInvokeException.FromException(ex);

return new Tuple<string, byte[]>(serializedHeader, responseMsgBody);
}
}
}
32 changes: 32 additions & 0 deletions test/Dapr.E2E.Test.Actors/RegressionActor/IRegression762Actor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// ------------------------------------------------------------------------
// Copyright 2021 The Dapr Authors
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
// http://www.apache.org/licenses/LICENSE-2.0
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// ------------------------------------------------------------------------

using System.Threading.Tasks;
using Dapr.Actors;

/// <summary>
/// This actor is being used to test the specific regression found in:
///
/// https://github.com/dapr/dotnet-sdk/issues/762
/// </summary>
namespace Dapr.E2E.Test.Actors.ErrorTesting
{
public interface IRegression762Actor : IPingActor, IActor
{
Task<string> GetState(string id);

Task SaveState(StateCall call);

Task RemoveState(string id);
}
}
22 changes: 22 additions & 0 deletions test/Dapr.E2E.Test.Actors/RegressionActor/StateCall.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// ------------------------------------------------------------------------
// Copyright 2021 The Dapr Authors
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
// http://www.apache.org/licenses/LICENSE-2.0
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// ------------------------------------------------------------------------

namespace Dapr.E2E.Test.Actors.ErrorTesting
{
public class StateCall
{
public string Key { get; set; }
public string Value { get; set; }
public string Operation { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
// limitations under the License.
// ------------------------------------------------------------------------

using System.Collections.Generic;
using System.Threading.Tasks;
using Dapr.Actors.Runtime;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFrameworks>netcoreapp3.1;net5</TargetFrameworks>
</PropertyGroup>

<PropertyGroup Condition=" '$(RunConfiguration)' == 'Dapr.E2E.Test.App.ReentrantActor' " />
<ItemGroup>
<ProjectReference Include="..\..\src\Dapr.AspNetCore\Dapr.AspNetCore.csproj" />
<ProjectReference Include="..\..\src\Dapr.Actors.AspNetCore\Dapr.Actors.AspNetCore.csproj" />
<ProjectReference Include="..\Dapr.E2E.Test.Actors\Dapr.E2E.Test.Actors.csproj" />
</ItemGroup>

</Project>
33 changes: 33 additions & 0 deletions test/Dapr.E2E.Test.App.ReentrantActor/Program.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// ------------------------------------------------------------------------
// Copyright 2021 The Dapr Authors
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
// http://www.apache.org/licenses/LICENSE-2.0
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// ------------------------------------------------------------------------

using Microsoft.AspNetCore.Hosting;
using Microsoft.Extensions.Hosting;

namespace Dapr.E2E.Test.App.ReentrantActors
{
public class Program
{
public static void Main(string[] args)
{
CreateHostBuilder(args).Build().Run();
}

public static IHostBuilder CreateHostBuilder(string[] args) =>
Host.CreateDefaultBuilder(args)
.ConfigureWebHostDefaults(webBuilder =>
{
webBuilder.UseStartup<Startup>();
});
}
}
51 changes: 51 additions & 0 deletions test/Dapr.E2E.Test.App.ReentrantActor/Startup.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// ------------------------------------------------------------------------
// Copyright 2021 The Dapr Authors
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
// http://www.apache.org/licenses/LICENSE-2.0
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// ------------------------------------------------------------------------

using Dapr.E2E.Test.Actors.Reentrancy;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;

namespace Dapr.E2E.Test.App.ReentrantActors
{
public class Startup
{
// This method gets called by the runtime. Use this method to add services to the container.
// For more information on how to configure your application, visit https://go.microsoft.com/fwlink/?LinkID=398940
public void ConfigureServices(IServiceCollection services)
{
services.AddActors(options =>
{
options.Actors.RegisterActor<ReentrantActor>();
options.ReentrancyConfig = new() { Enabled = true };
});
}

// This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
{
if (env.IsDevelopment())
{
app.UseDeveloperExceptionPage();
}

app.UseRouting();

app.UseEndpoints(endpoints =>
{
endpoints.MapActorsHandlers();
});
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"Logging": {
"LogLevel": {
"Default": "Information",
"Microsoft": "Warning",
"Microsoft.Hosting.Lifetime": "Information"
}
}
}
10 changes: 10 additions & 0 deletions test/Dapr.E2E.Test.App.ReentrantActor/appsettings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"Logging": {
"LogLevel": {
"Default": "Information",
"Microsoft": "Warning",
"Microsoft.Hosting.Lifetime": "Information"
}
},
"AllowedHosts": "*"
}
65 changes: 65 additions & 0 deletions test/Dapr.E2E.Test.App/Actors/Regression762Actor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// ------------------------------------------------------------------------
// Copyright 2021 The Dapr Authors
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
// http://www.apache.org/licenses/LICENSE-2.0
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// ------------------------------------------------------------------------

using System;
using System.Threading.Tasks;
using Dapr.Actors.Runtime;
using Dapr.E2E.Test.Actors.ErrorTesting;

namespace Dapr.E2E.Test.App.ErrorTesting
{
public class Regression762Actor : Actor, IRegression762Actor
{
public Regression762Actor(ActorHost host) : base(host)
{
}

public Task Ping()
{
return Task.CompletedTask;
}

public async Task<string> GetState(string id)
{
var data = await this.StateManager.TryGetStateAsync<string>(id);

if (data.HasValue)
{
return data.Value;
}
return string.Empty;
}

public async Task RemoveState(string id)
{
await this.StateManager.TryRemoveStateAsync(id);
}

public async Task SaveState(StateCall call)
{
if (call.Operation == "ThrowException")
{
await this.StateManager.SetStateAsync<string>(call.Key, call.Value);
throw new NotImplementedException();
}
else if (call.Operation == "SetState")
{
await this.StateManager.SetStateAsync<string>(call.Key, call.Value);
}
else if (call.Operation == "SaveState")
{
await this.StateManager.SaveStateAsync();
}
}
}
}
Loading

0 comments on commit 764b4d7

Please sign in to comment.