evanderkoogh / otel-cf-workers

An OpenTelemetry compatible library for instrumenting and exporting traces for Cloudflare Workers
BSD 3-Clause "New" or "Revised" License
208 stars 39 forks source link

CF browser rendering breaks when using instrumentDO #63

Closed jakst closed 8 months ago

jakst commented 9 months ago

I have a Durable Object in which I run Cloudflare's browser rendering beta. When wrapping the DO with instrumentDO the call to puppeteer.launch(this.env.BROWSER) crashes with the following error:

TypeError: Invalid URL string.
    at Object.apply (file:///private/var/folders/7y/mzdgqpln5w58j8cb5zf09nn80000gn/T/tmp-35961-0Gm7slF7r3UZ/worker.js:18343:20)
    at proxyHandler.apply (file:///private/var/folders/7y/mzdgqpln5w58j8cb5zf09nn80000gn/T/tmp-35961-0Gm7slF7r3UZ/worker.js:16921:22)
    at PuppeteerWorkers.launch (file:///private/var/folders/7y/mzdgqpln5w58j8cb5zf09nn80000gn/T/tmp-35961-0Gm7slF7r3UZ/worker.js:13744:32)

I managed to create a very hacky solution by patching this package in the following way:

node_modules/@microlabs/otel-cf-workers/dist/esm/wrap.js

export function wrap(item, handler, autoPassthrough = true) {
    if (isWrapped(item) || !isProxyable(item)) {
        return item;
    }
    const proxyHandler = Object.assign({}, handler);
    proxyHandler.get = (target, prop, receiver) => {
+   if (prop ==='BROWSER') return target.BROWSER
        if (prop === unwrapSymbol) {
            return item;
        }

This works for me for now, but I suspect there is a much better way to solve this in a first hand way.

evanderkoogh commented 9 months ago

That is no good.. I haven’t looked at the rendering beta yet. But is env.BROWSER a url as a string?

jakst commented 9 months ago

No it's an instantiated class I believe. puppeteer.launch(endpoint: { fetch: typeof fetch }) is the signature, so I guess there's a fetch function in there.

jakst commented 9 months ago

Here's the code in the @cloudflare/puppeteer package https://github.com/cloudflare/puppeteer/blob/35c54456f9f75eb2028569c51a269bbf22aad605/src/puppeteer-core.ts#L65

evanderkoogh commented 9 months ago

Hmm.. in that case it might confuse it with a service binding and something go wrong from there. I am currently on PTO, so probably won’t have time to look at it for a couple days.

jakst commented 9 months ago

That's alright, it's for a hobby project and I've patched your package with my workaround locally, so it's running fine for now. This will only start becoming painful if you start releasing a lot of new versions without fixing the bug ^^

No stress, and enjoy your time off!