Skip to content
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

Broken OpenAI LLM implementation for node version 18 #1010

Closed
DarylRodrigo opened this issue Apr 27, 2023 · 10 comments · Fixed by #1222
Closed

Broken OpenAI LLM implementation for node version 18 #1010

DarylRodrigo opened this issue Apr 27, 2023 · 10 comments · Fixed by #1222
Labels
question Further information is requested

Comments

@DarylRodrigo
Copy link

Issue

When running the vanilla OpenAI LLM function, it returns a 400 for node version 18.

To reproduce

The following code was used to create this error.

import { OpenAI } from "langchain/llms/openai";

export const run = async () => {
  const model = new OpenAI({ modelName: "gpt-3.5-turbo"});
  try {

    const resA = await model.call(
      "What would be a good company name a company that makes colorful socks?"
    );
    console.log({ resA });
    } catch (e: any) {
      console.log(e!.response!.data)
    }
};

run()

Error from the response body

{
  error: {
    message: 'you must provide a model parameter',
    type: 'invalid_request_error',
    param: null,
    code: null
  }
}

As seen, even though a model parameter is provided it still returns an error asking to provide one.

Additional notes

This issue is not present in node v19 and v20.

@JeremyFabrikapp
Copy link
Contributor

I've been struggling with that one, thanks for the issue report.

JeremyFabrikapp added a commit to JeremyFabrikapp/langchainjs that referenced this issue Apr 28, 2023
This solve an issue described here langchain-ai#1010
This was referenced Apr 28, 2023
@kwkr
Copy link
Contributor

kwkr commented Apr 28, 2023

@DarylRodrigo
I just run this code using v18.15.0 and it works. The only change to the code you provided was adding openAIApiKey to the constructor. Do you have any more details on this?

@nfcampos
Copy link
Collaborator

@DarylRodrigo @JeremyFabrikapp I just ran the exact code snippet above using node v18.16.0 and it works. Can you confirm the exact version of Node you're using? And any more details about your setup

nfcampos pushed a commit that referenced this issue Apr 28, 2023
This solve an issue described here #1010
@nfcampos nfcampos added the question Further information is requested label May 1, 2023
@JeremyFabrikapp
Copy link
Contributor

@nfcampos it seems like the issue doesn't occur on every 18.x version, i've tried the followings and so far found only one incompatibility :

  • Node v18.0.0 (npm v8.6.0) ✅
  • Node v18.1.0 (npm v8.8.0) ❌
  • Node v18.4.0 (npm v8.12.1) ✅
  • Node v18.16.0 (npm v9.5.1) ✅

I'm using NVM.
I can push a fix to .nvmrc by setting specifically 18.16.0 rather than v18 or 20.

@jacobcwright
Copy link

Using Node v18.16.0 and still getting same issue

Error: Request failed with status code 400 and body {
    "error": {
        "message": "you must provide a model parameter",
        "type": "invalid_request_error",
        "param": null,
        "code": null
    }
}

@nfcampos
Copy link
Collaborator

nfcampos commented May 9, 2023

@dqbd another one to look into, I cannot reproduce this

@JeremyFabrikapp
Copy link
Contributor

@jacobcwright can you confirm it's working on your side with node 20.x ?
Also are you using a static Node install or nvm ?

@jacobcwright
Copy link

jacobcwright commented May 9, 2023

I'm using nvm
It does indeed work with Node v20.1.0
@JeremyFabrikapp

@dqbd
Copy link
Collaborator

dqbd commented May 11, 2023

The issue stems from fetch implementation found in Node 18.1.0 adding an additional suffix to the Content-Type when passing a string as a body. The Content-Type header will thus be application/json, text/plain;charset=UTF-8 instead of application/json.

The issue seems to be gone in LangchainJS 0.0.71 and later, where the fetch adapter is not used when NodeJS is detected #1144 and the issue goes away when using other NodeJS 18.x versions. Consider upgrading either of them to fix this issue.

Closing this issue for now.


Note: if the issue occurs again, it might make sense to patch axios-fetch-adapter to convert body into an Uint8Array instead, this will fix the issue even in NodeJS 18.1.0, but I'm not sure if this might break other environments as well.

@JeremyFabrikapp
Copy link
Contributor

Thanks @dqbd for the coverage!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants