Not working in Cloudflare worker: Error: Code generation from strings disallowed for this context, tough-cookie

[ManasMadrecha]

Bug Report

Describe the bug

I was using this amazing lib in Firebase Functions, but I have recently moved to Cloudflare Workers. But, I am getting Code generation from strings disallowed for this context error when I use in lib in production api.

Note: I have enabled nodejs in cloudflare. nodejs_compat flag in the worker but still this error comes with this lib.

Minimal Reproduction

• latest Nitro app (unjs/nitro) with preset cloudflare-module

Environment

Browser or Node: Cloudflare (with nodejs_compat enabled)
Node version (if applicable): on dev (windows 11), node version 20+
Npm version: 10+

Additional Context

Docs

Cloudflare docs (https://developers.cloudflare.com/workers/runtime-apis/web-standards/#javascript-standards) say:

eval() is not allowed for security reasons.
new Function is not allowed for security reasons.

Is yf lib using these? If yes, is there a way to refactor them?

[EDIT] Error

I think the cause of error might be a particular package tough-cookie, because when I build my nitro app with yf lib, this non-fatal error shown in terminal (though the app still builds, but api routes using yf lib dont work):

(node-resolve plugin) Could not resolve import "unenv/runtime/mock/proxy-cjs/" in C:\Users\<user/nitroapp>\api\node_modules\tough-cookie\lib\cookie.js using exports defined in C:\Users\<user/nitroapp>\api\node_modules\unenv\package.json.

(node:13008) [DEP0155] DeprecationWarning: Use of deprecated trailing slash pattern mapping "./runtime/mock/proxy-cjs/" in the "exports" field module resolution of the package at C:\Users\<user/nitroapp>\api\node_modules\unenv\package.json imported from C:\Users\<user/nitroapp>\api\node_modules. Mapping specifiers ending in "/" is no longer supported.

(Use `node --trace-deprecation ...` to show where the warning was created)

[eddie-atkinson]

Just to add some colour to this issue I think the problem (or at least one of them) is coming from the dependency on ajv for validation.

Running the repro env using nitro and wrangler I get the same error message as in the AJV issue:

Code generation from strings disallowed for this context

There are also possibly more issues with getting this to run in a non-node runtime environment.

Reading through the source code ajv is clearly a core part of this library, so ideally we'd fix the issue at source rather than here. That being said, I've read through the source for ajv where this issue appears to be coming from and it's hairy as hell.

I'd happily contribute PRs to rewrite the validation logic in this library to another strategy like zod, but am not sure I personally could solve this problem upstream.

I also appreciate that it's not a stated goal of this library to run on every JS runtime, so if the answer is "too much complexity" or "give the AJV people a chance" I'd also understand.

Any thoughts @gadicc?

[gadicc]

Hey @eddie-atkinson, thanks for this report too.

1. I would love for the library to run in non-node environments. I personally use edge computing on a number of other projects so I'm totally behind this in principle.

2. However, you're right, ajv is very central to the library and won't be easy to replace. We don't actually run any complex validation of the kind zod does so well. The flow is actually: a) typescript interfaces for all the APIs, b) typescript interfaces -> json schema, c) ajv to validate input using the json schema. But to further complicate matters, we use a number of ajv features / API for a bunch of more complicated cases specific to the yahoo API responses. Lastly, AJV was actually selected (at the time?) due to being significantly faster (by a large margin) than all other solutions, which is handy considering that we have 1000's of tests.

Having said that, I've also had issues debugging ajv before, and I think actually we're still stuck on an old release (at least for a while, I haven't checked recently) because something broke and it was so hard to work out what was happening. I also think Zod has done a great job at establishing itself, and of course, we can infer the typescript interfaces from zod schemas, it could even uncomplicate our release logic to not need to always rebuild the schema. But this would be a huuuge job, and I also think, probably worthy of a communal debate with enough time to elicit responses.

If after all that, it's still something you'd like to investigate, you have my support :) I would probably start off by looking for something that converts typescript interfaces to zod code, as a starting point... as there are a loooot of them (see e.g. quoteSummary-iface.ts).

[eddie-atkinson]

Hi @gadicc,

Thank you for taking the time to give a detailed response.

The flow is actually: a) typescript interfaces for all the APIs, b) typescript interfaces -> json schema, c) ajv to validate input using the json schema.

Given that this is the current workflow it feels like cutting out JSON Schema could make sense as a way of simplifying development as the source of truth is Typescript.

Lastly, AJV was actually selected (at the time?) due to being significantly faster (by a large margin) than all other solutions, which is handy considering that we have 1000's of tests

That's a fair call out that I didn't consider. Looking at the benchmarking done here it seems like zod has pretty poor performance characteristics compared to ajv. Interestingly the top 3 in the rankings all use Typescript types as their source of truth, though admittedly not "pure" Typescript (they have annotations you need to add). I did a little survey of the tools that ranked higher than ajv on that benchmark and I'm interested in your view. From the perspective of bundle size, maturity, and weekly downloads @sinclair/typebox seems like a promising option. Especially given that its perf in AOT and JIT mode exceed ajv. It also appears to support CF workers.

(source)

Name	Stars	Weekly Downloads	Date of first commit	Minified Bundle Size (as reported by bundlephobia)
ts-runtime-checks	228	146	2022-07-22	58 kB
@sinclair/typebox	4200	24,239,331	2017-04-07	50 kB
typia	4045	24,148	2022-04-19	65.4 kB
spectypes	91	31	2022-04-29	1.7 kB
suretype	490	17,553	2023-02-26	139 kB
ts-json-validator	341	29,848	2019-10-31	118.8 kB
ajv	13,378	81,611,055	2015-05-31	119.6 kB

But this would be a huuuge job, and I also think, probably worthy of a communal debate with enough time to elicit responses.

Is there a particular venue where you'd like to have that discussion? Here in this thread? GitHub discussions? I appreciate that this thread is, at the moment at least, a bit too hidden a way to have a discussion of this magnitude.

I would probably start off by looking for something that converts typescript interfaces to zod code, as a starting point... as there are a loooot of them

Yes and no, as shown by my highly scientific survey here:

The real questions for me would be less the leg work of converting all the interfaces and more so how we review the changes.

Keen to hear your thoughts and happy to put together a micro benchmark of some of the more gnarly interfaces to test feasibility and performance on any libraries of interest.

[gadicc]

Is there a particular venue where you'd like to have that discussion? Here in this thread? GitHub discussions? I appreciate that this thread is, at the moment at least, a bit too hidden a way to have a discussion of this magnitude.

Agree. Let's continue on the new issue above. GitHub discussions is also a bit too hidden. I'll reply to you there now and later this week I'll tag recent contributors in case they have any input.

[gadicc]

Happy to report that @eddie-atkinson's hero work in #772 has been merged. We longer have a dependency on ajv.

I'm not sure about latest version of tough-cookie or anything else, maybe someone else can report back.

NB: the PR has been merged to devel but has not been published yet.

[eddie-atkinson]

So I've been playing around with this locally using the version of the code on devel.

I got it working, but had to tweak yahoo-finance2 itself in a few places:

1. In env-node.ts I had to remove the line import { URLSearchParams } from "url"; and simply use the global URLSearchParams. I couldn't fix this by changing this to node:url either which is annoying. This should be fixable just need to conditionally import the value
2. process is not a global in the Workers runtime so I had to change this in two places to import * as process from "node:process";. This would be fine to change except this import does not work with bare process and requires the node: prefix, which is supported as of node 18. This package supports 16 and above, so no dice there
3. In yahooFinanceFetch on line 137 we're doing const setCookieHeaders = response.headers.raw()["set-cookie"];. The response headers from CloudFlare's fetch don't have a .raw() method, and also don't have a set-cookie header as far as I can see. I just hard coded this line to false and then everything works

So, in other words we've made some progress! Though still a couple of PRs to go before we can call this mission success.

I'd love to get this working, but we need to think pretty carefully about how to keep this library agnostic to the runtime it targets. The last thing I'd like to do is add weird specific code for each runtime

[gadicc]

Hey @eddie-atkinson, always great to have your input here, thanks. Happy we haven't burnt you out yet ;)

we need to think pretty carefully about how to keep this library agnostic to the runtime it targets. The last thing I'd like to do is add weird specific code for each runtime

Exactly! But I think you're off to a great start. And the issues you've discovered all look pretty minor.

1. Actually it looks like URLSearchParams is available globally since Node 10, so that seems fine. Wow, how old is yf2??? 😅

2. I just checked the node release matrix (for the first time in a while) and am happy to drop support for node <= 18 which are all out of maintenance period now.

3. Not familiar with Cloudflare workers specifically but I'll assume they have everything that regular Web Workers have, in which case, we'll have access to Headers#getSetCookie().

Some other notes:

a) We could also look at exactly what's being imported and if that's correct, e.g. maybe we shouldn't be loading the node environment at all outside of node. Should we be using index-browser.ts env? I don't know the answer, we need to look at it, but in any case, maybe irrelevant if the 3 points above are the only blockers.

b) Related though, in index-node.ts env we use the node-fetch package which is actually what's providing headers.raw(). I mean, it's underlying deps won't exist in non-node, so, we can't go this route anyway, but worth mentioning as its related to the previous point.

c) Depending on when you last pulled devel, I made a few more changes in the last couple of days.

Anyways, let's see how we go. Thanks for taking a look into this, and so soon after your your other work!

[gadicc]

Ah, also, re (3), some maybe useful code from another project of mine.

https://github.com/gadicc/jest-fetch-mock-cache/blob/57fd9c30c7ae1b989502b4f62019e246ce91c94a/src/headers.ts#L8-L25

[eddie-atkinson]

Hi @gadicc,

I've opened a PR for the issues I call out above. Thank you once again for your thoughtful response and in-depth feedback.

Actually it looks like URLSearchParams is available globally since Node 10, so that seems fine. Wow, how old is yf2??? 😅

Jurassic apparently, I'm using the global in the PR.

I just checked the node release matrix (for the first time in a while) and am happy to drop support for node <= 18 which are all out of maintenance period now.
BAM and the Node 16 support is gone (in the PR)

Not familiar with Cloudflare workers specifically but I'll assume they have everything that regular Web Workers have, in which case, we'll have access to Headers#getSetCookie().

Now that we only support Node 18 and up I just dropped node-fetch entirely and switched to the standard library fetch implementation which became standard in Node 18, one more dependency down

maybe irrelevant if the 3 points above are the only blockers.

Running against my branch I managed to get a CloudFlare worker returning yf.quote("AAPL"). So I think we're good for now, though worth thinking about.

[gadicc]

Hey @eddie-atkinson, thanks so much. Was AFK over the weekend, will review now. Very exciting to finally be able to drop node-fetch (despite how well it's served us all these years, I'm very grateful of course 🙏), but yes, so many things I've wanted to drop for years which we finally can now 🎉 I'll maybe even announce upcoming end of life for CJS, which is a burden too. Anyways, we'll chat more in the PR.