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.

Sign up for GitHub

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

feat: Adds error handling around actor deactivation #628

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat: Adds error handling around actor deactivation #628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 23 additions & 13 deletions src/actors/runtime/ActorManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ limitations under the License.
*/

import DaprClient from "../../implementation/Client/DaprClient";
import { Logger } from "../../logger/Logger";
import Class from "../../types/Class";
import ActorId from "../ActorId";
import AbstractActor from "./AbstractActor";
Expand All @@ -23,11 +24,17 @@ import BufferSerializer from "./BufferSerializer";
* The Actor Manager manages actor objects of a specific actor type
*/
const REMINDER_METHOD_NAME = "receiveReminder"; // the callback method name for the reminder
export enum DeactivateResult {
Success,
ActorDoesNotExist,
Error,
}

export default class ActorManager<T extends AbstractActor> {
readonly actorCls: Class<T>;
readonly daprClient: DaprClient;
readonly serializer: BufferSerializer = new BufferSerializer();
readonly logger: Logger;

actors: Map<string, T>;

Expand All @@ -39,6 +46,7 @@ export default class ActorManager<T extends AbstractActor> {
this.daprClient = daprClient;
this.actorCls = actorCls;

this.logger = new Logger("Actors", "ActorManager", daprClient.options.logger);
this.actors = new Map<string, T>();

// @todo: we need to make sure race condition cannot happen when accessing the active actors
Expand Down Expand Up @@ -71,20 +79,22 @@ export default class ActorManager<T extends AbstractActor> {
this.actors.set(actorId.getId(), actor);
}

async deactivateActor(actorId: ActorId): Promise<void> {
if (!this.actors.has(actorId.getId())) {
throw new Error(
JSON.stringify({
error: "ACTOR_NOT_ACTIVATED",
errorMsg: `The actor ${actorId.getId()} was not activated`,
}),
);
async deactivateActor(actorId: ActorId): Promise<DeactivateResult> {
if (this.actors.has(actorId.getId())) {
try {
const actor = await this.getActiveActor(actorId);
await actor.onDeactivateInternal();
return DeactivateResult.Success;
} catch (error) {
this.logger.error("Error encountered deactivating actor");
} finally {
this.actors.delete(actorId.getId());
}
return DeactivateResult.Error;
} else {
this.logger.warn(`The actor ${actorId.getId()} was not activated`);
return DeactivateResult.ActorDoesNotExist;
}

const actor = await this.getActiveActor(actorId);
await actor.onDeactivateInternal();

this.actors.delete(actorId.getId());
}

async getActiveActor(actorId: ActorId): Promise<T> {
Expand Down
4 changes: 2 additions & 2 deletions src/actors/runtime/ActorRuntime.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { ActorRuntimeOptions } from "../../types/actors/ActorRuntimeOptions";
import Class from "../../types/Class";
import ActorId from "../ActorId";
import AbstractActor from "./AbstractActor";
import ActorManager from "./ActorManager";
import ActorManager, { DeactivateResult } from "./ActorManager";

/**
* Creates instances of "Actor" and activates and deactivates "Actor"
Expand Down Expand Up @@ -141,7 +141,7 @@ export default class ActorRuntime {
return await manager.fireTimer(actorIdObj, name, requestBody);
}

async deactivate(actorTypeName: string, actorId: string): Promise<void> {
async deactivate(actorTypeName: string, actorId: string): Promise<DeactivateResult> {
const actorIdObj = new ActorId(actorId);
const manager = this.getActorManager(actorTypeName);
return await manager.deactivateActor(actorIdObj);
Expand Down
17 changes: 15 additions & 2 deletions src/implementation/Server/HTTPServer/actor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { DaprClient } from "../../..";
import { Logger } from "../../../logger/Logger";
import { getRegisteredActorResponse } from "../../../utils/Actors.util";
import HttpStatusCode from "../../../enum/HttpStatusCode.enum";
import { DeactivateResult } from "../../../actors/runtime/ActorManager";

// https://docs.dapr.io/reference/api/bindings_api/
export default class HTTPServerActor implements IServerActor {
Expand Down Expand Up @@ -89,8 +90,20 @@ export default class HTTPServerActor implements IServerActor {
private async handlerDeactivate(req: IRequest, res: IResponse): Promise<IResponse> {
const { actorTypeName, actorId } = req.params;
const result = await ActorRuntime.getInstance(this.client.daprClient).deactivate(actorTypeName, actorId);
res.statusCode = HttpStatusCode.OK;
return this.handleResult(res, result);

switch (result) {
case DeactivateResult.Success:
res.statusCode = HttpStatusCode.OK;
return this.handleResult(res, result);
case DeactivateResult.Error:
res.statusCode = HttpStatusCode.INTERNAL_SERVER_ERROR;
Copy link
Author

Choose a reason for hiding this comment

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

Does returning specific http status codes for the unhappy path provide value to the dapr sidecar?

return this.handleResult(res, result);
case DeactivateResult.ActorDoesNotExist:
res.statusCode = HttpStatusCode.NOT_FOUND;
return this.handleResult(res, result);
default:
throw new Error("Unsupported result type received");
}
}

private async handlerMethod(req: IRequest, res: IResponse): Promise<IResponse> {
Expand Down
20 changes: 20 additions & 0 deletions test/unit/actor/actorRuntime.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import DemoActorCounterImpl from "../../actor/DemoActorCounterImpl";
import DemoActorSayImpl from "../../actor/DemoActorSayImpl";
import { ActorRuntimeOptions } from "../../../src/types/actors/ActorRuntimeOptions";
import { randomUUID } from "crypto";
import { DeactivateResult } from "../../../src/actors/runtime/ActorManager";

describe("ActorRuntime", () => {
let client: DaprClient;
Expand Down Expand Up @@ -110,6 +111,25 @@ describe("ActorRuntime", () => {
expect(res.toString()).toEqual(`Actor said: "Hello World"`);
});

it("should be able to deactivate a actor", async () => {
const actorId = randomUUID();

await runtime.registerActor(DemoActorSayImpl);
await runtime.invoke(DemoActorSayImpl.name, actorId, "sayString", Buffer.from("Hello World"));
const result = await runtime.deactivate(DemoActorSayImpl.name, actorId);

expect(result).toEqual(DeactivateResult.Success);
});

it("should not error when calling deactivate on an actor that does not exist", async () => {
const actorId = randomUUID();

await runtime.registerActor(DemoActorSayImpl);
const result = await runtime.deactivate(DemoActorSayImpl.name, actorId);

expect(result).toEqual(DeactivateResult.ActorDoesNotExist);
});

it("should receive an error if the actor method does not exist", async () => {
const actorId = randomUUID();

Expand Down
Loading