temporalio / sdk-typescript

Temporal TypeScript SDK
Other
535 stars 106 forks source link

[Proposal] Built-in registered handlers query #1081

Open josiah-roberts opened 1 year ago

josiah-roberts commented 1 year ago

Link to thread with @bergundy

Is your feature request related to a problem? Please describe.

There's a common need to keep track of what queries and signals are legal to send to a workflow at this moment. My team is creating a layer for this internally, but it would be ideal to solve it on a SDK level.

Describe the solution you'd like

I'd like to see a built-in query which returns the queries and signals being currently listened to. This could take two forms:

Separate

Together

In either case, we can't preserve the type information about these queries or signals, but we can at least maintain the knowledge of whether there is an active setHandler(name, () => {...}) for a given name. This allows us to drive UX experiences that only allow currently legal operations, which is a powerful technique for code-as-source-of-truth on our workflows.

Additional context

Here's a spike implementation for a trackHandler shim over setHandler that we're working on internally. Workflows need to be wrapped with trackHandlers in the form export const myWorkflow = (args: Args) => trackHandlers(myWorkflowImpl, args);. If this was built-in, such wrapping would not be needed.

const trackedHandlerStorage = new AsyncLocalStorage<Set<string>>();
const getTrackedHandlers = () => {
  const handlers = trackedHandlerStorage.getStore();
  if (!handlers) {
    throw new IllegalStateError('Not in a tracked context!');
  }
  return handlers;
};

export const trackHandler: typeof setHandler = (def, handler) => {
  const handlers = getTrackedHandlers();
  handlers.add(def.name);
  setHandler(def, ((...args) => {
    trackedHandlerStorage.enterWith(handlers);
    if (handler) {
      return handler(...(args as Parameters<typeof handler>));
    }
    return undefined;
  }) as typeof handler);
};

export const clearHandler = (def: Parameters<typeof setHandler>[0]) => {
  const handlers = getTrackedHandlers();
  handlers.delete(def.name);
  setHandler(def, undefined);
};

export const clearHandlers = (defs: Array<Parameters<typeof setHandler>[0]>) => {
  for (const def of defs) {
    clearHandler(def);
  }
};

export const handlersQuery = defineQuery<string[]>('handlers');
export const trackHandlers = <R, TArgs extends any[]>(
  callback: (...args: TArgs) => R,
  ...args: TArgs
): R =>
  trackedHandlerStorage.run(new Set(), () => {
    const handlersRef = getTrackedHandlers();
    trackHandler(handlersQuery, () => [...handlersRef]);
    return callback(...args);
  });
bergundy commented 1 year ago

Thank you for this, I’m mostly afk until the end of the week. I’ll take some time to think about this and respond.

bergundy commented 1 year ago

Haven't forgotten about this. I want @cretz to review the response structure for the built-in query because we want this cross SDKs and he's really good and opinionated at API design.

cretz commented 1 year ago

I made a comment at https://github.com/temporalio/features/issues/51#issuecomment-1485273243

Even though it contains a lot of details, can ignore most and just impl the "get workflow metadata" query to start with. We do need this as protos in https://github.com/temporalio/api/tree/master/temporal/api/sdk/v1 though, not just TS definitions.

mjameswh commented 1 month ago

@antlai-temporal Can we close this ticket? I believe you implemented that in #1319. Anything left here?