workerd icon indicating copy to clipboard operation
workerd copied to clipboard

🐛 Bug: WorkerEntrypoint wrapped with Proxy throws error when invoked

Open rtbenfield opened this issue 1 year ago • 6 comments

A WorkerEntrypoint class implementation that is wrapped in a Proxy throws the following runtime error when the service binding is invoked. This was created in a Worker with a service binding to its own entrypoint for testing.

✘ [ERROR] workerd/io/worker.c++:1819: error: worker is not an actor but class name was requested; n = RpcEntrypoint

class RpcEntrypointImpl extends WorkerEntrypoint {
	myRpcCall() {
		return { ok: true }
	}
}
export const RpcEntrypoint = new Proxy(RpcEntrypointImpl, {})

// logs RpcEntrypoint__proto__ [Function: WorkerEntrypoint]
console.log('RpcEntrypoint__proto__', RpcEntrypoint.__proto__)

I found the error at the line below and attempted to trace how WorkerEntrypoint exports are determined. From what I understand, it should be detecting from __proto__, though in the JS side this seems to be retained by the proxy wrapper. This is my first time digging through the workerd code though so I could be off track. https://github.com/cloudflare/workerd/blob/92c30c97870551bb2954c77ba69c419da0c7a058/src/workerd/io/worker.c%2B%2B#L1819

Why proxy a WorkerEntrypoint?

I'm attempting the Proxy wrapper as a way to apply OpenTelemetry instrumentation to a WorkerEntrypoint class. The proxy would extract the env form the constructor for configuration, wrap all RPC method invocations in spans, and export those spans automatically when the RPC method completes.

rtbenfield avatar Jun 19 '24 15:06 rtbenfield

Is there a way you can accomplish this without using a Proxy?

irvinebroque avatar Jul 16 '24 18:07 irvinebroque

seems like what you're trying to do is kind of equivalent to applying a decorator to all the different RPC methods?

irvinebroque avatar Jul 16 '24 18:07 irvinebroque

Yeah, applying a decorator is essentially the goal. The Proxy approach is what @microlabs/otel-cf-workers is currently using for all its instrumentation. It makes it simple to apply the decorator to all the methods and automatically instrument the env bindings.

It may be possible to accomplish this with JS decorators on the method level, though I would expect a class-level decorator to have the same problem and need a Proxy to work. So one would have to apply the decorator annotation to every method on the class. My understanding is that JS decorators are not in the runtime yet, so we'd also have to do some additional transpiling.

I'm also thinking that the next step is distributed tracing through RPC. That would need us to pass some OTEL data along with each RPC call. A decorator on the RPC method and another wrapping the bindings that provide RPC methods or stubs would be able to automatically inject and intercept that OTEL data. I haven't gotten that far yet, though I don't expect the outbound RPC call to have issues.

Do you have any other suggestions for accomplishing this without a Proxy that would match with the runtime's requirements? I was surprised that Proxy didn't work.

rtbenfield avatar Jul 16 '24 19:07 rtbenfield

With a quick investigation this definitely would appear to be a bug either on our part or in v8. Will investigate further

jasnell avatar Jul 16 '24 19:07 jasnell

Should be fixed by #2406 in the next release.

kentonv avatar Jul 22 '24 16:07 kentonv

That's great! Thanks y'all!

rtbenfield avatar Jul 22 '24 19:07 rtbenfield

Hey y'all 👋🏻 I noticed this was still open. The fixes for WorkerEntrypoint are working great. I think this can be closed now and wanted to check if there was a reason it is still open before closing it myself.

rtbenfield avatar Nov 28 '24 13:11 rtbenfield