-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
getEnvironmentVariable utility always return undefined on Cloudflare Workers #5303
Comments
To address the issue with export function getEnvironmentVariable(name: string): string | undefined {
try {
// Attempt to access the environment variable from the global scope
if (typeof globalThis[name] !== "undefined") {
return globalThis[name];
} else if (typeof process !== "undefined") {
// Fallback for Node.js environments
return process.env?.[name];
} else {
return undefined;
}
} catch (e) {
return undefined;
}
} This code first attempts to retrieve the environment variable from Regarding your suggestion to modify the model constructor for tracing properties, it's indeed a viable approach for passing configuration directly, enhancing flexibility and security by decoupling API keys and other sensitive information from your codebase. Implementing such a feature would involve extending the constructor's parameters to accept these additional properties, ensuring they are properly handled within the library. Before deploying these changes, ensure thorough testing in both Cloudflare Workers and Node.js environments to confirm that the modified
|
Hmm yeah that's not great - as a stopgap you can manually pass in a tracer like this: https://docs.smith.langchain.com/how_to_guides/tracing/trace_with_langchain#trace-selectively Will have a think to see how to do this more seamlessly CC @craigsdennis |
The other thing you could try (suggested by @efriis) is to manually set https://developers.cloudflare.com/workers/runtime-apis/nodejs/process/#processenv |
I understand that manually defining |
Would this work? import * as process from 'node:process';
process.env = env;
// tracing code |
Implementing that piece of code in the utility https://github.com/langchain-ai/langchainjs/blob/main/langchain-core/src/utils/env.ts would solve the problem. However, if I implement it myself in my project, it wouldn't solve anything, since using the |
To address this issue, we cannot simply adjust
const llm = new ChatOpenAI({
…otherOptions,
configuration: {
baseURL: this.env.CLOUDFLARE_AI_GATEWAY_OPENAI_ENDPOINT,
externalEnvs: env, // env is the environment variables injected into their runtime environment by other platforms (cloudflare, vercel)
},
}); (The |
I like it, it looks much cleaner than what I suggested before, thank you very much! |
Checked other resources
Example Code
https://github.com/langchain-ai/langchainjs/blob/main/langchain-core/src/utils/env.ts#L77
Error Message and Stack Trace (if applicable)
No response
Description
Greetings!
In Cloudflare Workers, the use of the API process requires to be imported, and the compatibility flag “nodejs_compat” must be used.
https://developers.cloudflare.com/workers/runtime-apis/nodejs/process/
However, in the utilities the “node:process” module is not imported, so referencing the API process generates an error and undefined is returned from the catch.
This problem doesn't allow LangSmith to be used from Cloudflare Workers.
I've been trying several solutions since I don't think it's a good idea to solve the problem by importing the “node:process” module, since that would force everyone to add the compatibility flag “nodejs_compat” in their workers.
Maybe the model constructor could receive some tracing props, in the same way as the verbose property, which can also be obtained from environment variables, something like that:
If you have any idea how we can solve this, I can create the feature!
System Info
platform Cloudflare Workers
The text was updated successfully, but these errors were encountered: