Implement `promiseHooks` from `node:v8`

[bkniffler]

What is the problem this feature would solve?

When trying to use temporal sdk (https://github.com/temporalio/sdk-typescript), getting error

node:v8 createHook is not yet implemented in Bun

Its used right here: https://github.com/temporalio/sdk-typescript/blob/3ce098be042e86d26ea5b433508a480503cb09b2/packages/worker/src/workflow/vm-shared.ts#L198

What is the feature you are proposing to solve the problem?

Implement createHook.

What alternatives have you considered?

Ask temporal to not use createHook, but probably out of reach since its needed to isolate the VM scope.

[Electroid]

We'd have to think about if this is worth implementing, since it could have a performance impact. (it tracks all promises) However, even if it is feasible, it's not a high priority.

[alirezabonab]

@Electroid Is there any update on this?

[danieldunderfelt]

Sadly the Temporal Typescript SDK does not work without createHook 😩

I'd love to have it at least as an opt-in if the performance impact is big.

Ping @lorensr

[loicnestler]

What's the status on this? Is an implementation of createHook planned?

[bergundy]

I'm the author of the code you linked in the Temporal TypeScript SDK. Promise hooks are optional, they allow us to capture the stack trace of all running functions in a workflow. I'm not sure if there's more of a gap for running our SDK on top of Bun but we can look into it. Apart from promise hooks, we use Node's vm and worker_threads modules, AsyncLocalStorage, and rely heavily on Rust Neon bindings.

[paradoxloop]

@bergundy if Promise Hooks are optional is there a way to disable them so that we can use Bun. There is a linked issue on the Temporal project.

[axe-me]

Can bun simply keep silent about this instead of throw an error, if there is no plan to implement this in short time. Just like what deno did in this PR: https://github.com/denoland/deno/pull/25979/files

maybe just use warnNotImplementedOnce in here: https://github.com/oven-sh/bun/blob/25fcbed8d187742faedfe2ff7ce413c059a3c774/src/js/node/v8.ts#L101-L117

or just simply remove the promiseHooks object

[bergundy]

#14485 seems like the way to go. If it ends up not getting merged, we may be able to do something on the Temporal SDK side, it should be easy to edit the SDK files post install to remove the use of promise hooks and confirm. I’m subscribed to this issue, wondering if this is the last hurdle for running the Temporal SDK on bun.

[cortopy]

@bergundy since this PR hasn't been merged yet, is there any chance that Temporal can do checks if the promiseHooks are available in the runtime?

Even if this was merged, the plans from Bun maintainers would be to warn in console that the API is not available, which wouldn't be ideal if Temporal API doesn't check for the API implementation in the first place

[bergundy]

I'm no longer maintaining the Temporal SDK but I will forward this request to @mjameswh.

[ngomezcn]

Has the situation progressed?

[mjameswh]

For information: the requirement on promiseHooks has been relaxed in Temporal SDK. The fix is currently on the main branch, it will be included in the next patch release (no ETA, but that will be in the next few days).

That is, we now ignore errors thrown while registering promiseHooks, and simply continue without support for __stack_trace and __enhanced_stack_trace queries.

Further investigation will be required to determine either there are other blockers to using Temporal SDK on Bun. Hopefully, we're getting close.