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(hub): add separate protocol for actor inspect #1946

Closed
wants to merge 1 commit into from
Closed

feat(hub): add separate protocol for actor inspect #1946

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
6 changes: 5 additions & 1 deletion frontend/apps/hub/src/domains/project/components/actors/actors-actor-details.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ export function ActorsActorDetails({
const { data } = useSuspenseQuery(actorQueryOptions(props));
return (
<ActorDetailsSettingsProvider>
<ActorWorkerContextProvider enabled={!data.destroyedAt} {...props}>
<ActorWorkerContextProvider
enabled={!data.destroyedAt}
endpoint={data.endpoint}
{...props}
>
Comment on lines +44 to +48
Copy link

Choose a reason for hiding this comment

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

style: endpoint prop is passed before spreading ...props which could lead to endpoint being overridden if it exists in props

<div className="flex flex-col h-full flex-1 pt-2">
<Tabs
value={tab || "logs"}
Expand Down
43 changes: 17 additions & 26 deletions frontend/apps/hub/src/domains/project/components/actors/worker/actor-repl.worker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { type ActorHandle, Client } from "@rivet-gg/actor-client";
import { ActorHandleRaw } from "@rivet-gg/actor-client";
import type { InspectRpcResponse } from "@rivet-gg/actor-protocol/ws/to_client";
import { fromJs } from "esast-util-from-js";
import { toJs } from "estree-util-to-js";
Expand Down Expand Up @@ -104,14 +104,7 @@ const createConsole = (id: string) => {
);
};

interface InspectableActor {
internal_setState(rpc: this, state: Record<string, unknown>): Promise<void>;
internal_inspect(): Promise<InspectRpcResponse>;
}

let init:
| null
| ({ handle: ActorHandle<InspectableActor> } & InspectRpcResponse);
let init: null | ({ handle: ActorHandleRaw } & InspectRpcResponse);

addEventListener("message", async (event) => {
const { success, data } = MessageSchema.safeParse(event.data);
Expand All @@ -130,11 +123,13 @@ addEventListener("message", async (event) => {
}

try {
const cl = new Client(data.managerUrl);
const handle = await Promise.race([
cl.getWithId<InspectableActor>(data.actorId, {}),
wait(5000).then(() => undefined),
]);
const handle = new ActorHandleRaw(
`${data.endpoint}/__inspect`,
undefined,
"cbor",
);

handle.connect();

if (!handle) {
respond({
Expand All @@ -146,7 +141,7 @@ addEventListener("message", async (event) => {
}

const inspect = await Promise.race([
handle.internal_inspect(),
handle.rpc<[], InspectRpcResponse>("inspect"),
wait(5000).then(() => undefined),
]);

Expand Down Expand Up @@ -208,21 +203,17 @@ addEventListener("message", async (event) => {
data: formatted,
});

const rpcs = Object.fromEntries(
actor.rpcs.map(
(rpc) =>
[
rpc,
actor.handle[rpc as keyof typeof actor.handle],
] as const,
),
const exposedActor = Object.fromEntries(
init?.rpcs.map((rpc) => [
rpc,
actor.handle.rpc.bind(actor.handle, rpc),
]) ?? [],
);

const evaluated = await evaluateCode(data.data, {
console: createConsole(data.id),
wait,
actor: actor.handle,
...rpcs,
actor: exposedActor,
});
return respond({
type: "result",
Expand Down Expand Up @@ -250,7 +241,7 @@ addEventListener("message", async (event) => {

try {
const state = JSON.parse(data.data);
await actor.handle.internal_setState(state);
await actor.handle.rpc("setState", state);
return respond({
type: "state-change",
data: {
Expand Down
19 changes: 4 additions & 15 deletions frontend/apps/hub/src/domains/project/components/actors/worker/actor-worker-container.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
import {
actorManagerUrlQueryOptions,
actorQueryOptions,
} from "@/domains/project/queries";
import { actorQueryOptions } from "@/domains/project/queries";
import { queryClient } from "@/queries/global";
import ActorWorker from "./actor-repl.worker?worker";
import {
Expand Down Expand Up @@ -66,11 +63,13 @@ export class ActorWorkerContainer {
projectNameId,
environmentNameId,
actorId,
endpoint,
signal,
}: {
projectNameId: string;
environmentNameId: string;
actorId: string;
endpoint: string;
signal: AbortSignal;
}) {
this.terminate();
Expand All @@ -79,16 +78,6 @@ export class ActorWorkerContainer {
this.#state.status = { type: "pending" };
this.#update();
try {
// To check if the actor is supported, first we need to get the actor manager URL
// if there's no manager URL, we can't support the actor
const managerUrl = await queryClient.fetchQuery(
actorManagerUrlQueryOptions({
projectNameId,
environmentNameId,
}),
);
signal.throwIfAborted();

// If we have the manager URL, next we need to check actor's runtime
const { actor } = await queryClient.fetchQuery(
actorQueryOptions({
Expand Down Expand Up @@ -118,7 +107,7 @@ export class ActorWorkerContainer {
const worker = new ActorWorker({ name: `actor-${actorId}` });
signal.throwIfAborted();
// now worker needs to check if the actor is supported
this.#setupWorker(worker, { actorId, managerUrl });
this.#setupWorker(worker, { actorId, endpoint });
signal.throwIfAborted();
return worker;
} catch (e) {
Expand Down
7 changes: 5 additions & 2 deletions frontend/apps/hub/src/domains/project/components/actors/worker/actor-worker-context.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ interface ActorWorkerContextProviderProps {
actorId: string;
projectNameId: string;
environmentNameId: string;
endpoint?: string;
enabled?: boolean;
children: ReactNode;
}
Expand All @@ -32,6 +33,7 @@ export const ActorWorkerContextProvider = ({
children,
actorId,
enabled,
endpoint,
projectNameId,
environmentNameId,
}: ActorWorkerContextProviderProps) => {
Expand All @@ -43,11 +45,12 @@ export const ActorWorkerContextProvider = ({
useEffect(() => {
const ctrl = new AbortController();

if (enabled) {
if (enabled && endpoint) {
container.init({
projectNameId,
environmentNameId,
actorId,
endpoint,
signal: ctrl.signal,
});
}
jog1t marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -56,7 +59,7 @@ export const ActorWorkerContextProvider = ({
ctrl.abort();
container.terminate();
};
}, [actorId, projectNameId, environmentNameId, enabled]);
}, [actorId, projectNameId, environmentNameId, endpoint, enabled]);

return (
<ActorWorkerContext.Provider value={container}>
Expand Down
2 changes: 1 addition & 1 deletion frontend/apps/hub/src/domains/project/components/actors/worker/actor-worker-schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const CodeMessageSchema = z.object({
});
const InitMessageSchema = z.object({
type: z.literal("init"),
managerUrl: z.string(),
endpoint: z.string(),
actorId: z.string(),
});

Expand Down
5 changes: 4 additions & 1 deletion frontend/apps/hub/src/domains/project/queries/actors/query-options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ export const actorQueryOptions = ({
(arg) => arg !== "",
),
},
endpoint: createActorEndpoint(data.actor.network),
}),
});
};
Expand Down Expand Up @@ -442,7 +443,9 @@ export const actorRegionQueryOptions = ({
});
};

const createActorEndpoint = (network: Rivet.actor.Network) => {
const createActorEndpoint = (
network: Rivet.actor.Network,
): string | undefined => {
const http = Object.values(network.ports).find(
(port) => port.protocol === "http" || port.protocol === "https",
);
Expand Down
6 changes: 3 additions & 3 deletions frontend/packages/components/src/ui/button.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
) => {
const C = asChild ? Slot : "button";

const isIcon = size?.includes("icon");

return (
<C
className={cn(
Expand All @@ -93,9 +95,7 @@ const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
) : startIcon ? (
React.cloneElement(startIcon, startIcon.props)
) : null}
{!size?.includes("icon") && isLoading ? null : (
<Slottable>{children}</Slottable>
)}
{isIcon && isLoading ? null : <Slottable>{children}</Slottable>}
Copy link

Choose a reason for hiding this comment

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

logic: This change may have introduced a regression. Previously, children were hidden for non-icon buttons during loading (!size?.includes('icon') && isLoading). Now children are only hidden for icon buttons during loading (isIcon && isLoading). This means non-icon buttons will show both the loading spinner and children simultaneously.

{endIcon ? React.cloneElement(endIcon, endIcon.props) : null}
</C>
);
Expand Down
Loading
Loading