fix(openai_api_compatible): fixing the error when converting chunk to json by aniaan · Pull Request #3570 · langgenius/dify

aniaan · 2024-04-17T11:37:11Z

Description

fixing the error when converting chunk to json

Fixes # (issue)

Type of Change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update, included: Dify Document
Improvement, including but not limited to code refactoring, performance optimization, and UI/UX improvement
Dependency upgrade

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

TODO

Suggested Checklist:

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
My changes generate no new warnings
I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods
optional I have made corresponding changes to the documentation
optional I have added tests that prove my fix is effective or that my feature works
optional New and existing unit tests pass locally with my changes

… JSON

bowenliang123 · 2024-04-17T13:05:18Z

As far as I know, the spaces at the front or the end of the string would not fail the json parsing.
Any further explanation or evidence for the issue you're fixing?

aniaan · 2024-04-18T03:28:22Z

chunk is not always a space. In some cases, it may be \r. When chunk='\r', an error will occur when executing the corresponding code.

This is the minimal simple reproducing case.

import json

chunk = '\r'
decoded_chunk = chunk.strip().lstrip('data: ').lstrip()
chunk_json = json.loads(decoded_chunk)

provide more explanation as to why sometimes the chunk will be \r. The reason is actually because the default delimiter for dify is \n\n, but I am using the library LLaMA-Factory, which uses the delimiter \r\n. When I first used dify, I configured it according to its default value \n\n, but there was no corresponding response from dify (actually, the program code has already thrown an error, but there was also a lack of logs), which made debugging difficult for me. So, I had to try changing some delimiters and added some code logs. When I tried using '\n' as the delimiter, although there were still errors, they could be resolved by adding one line of code chunk=chunk.strip(). That's why this pr exists.

There is no problem with using OpenAI's SDK by default to call LLaMA-Factory, so initially it made me suspect that the issue was with dify.

Regarding the use of \r\n in LLaMA-Factory, I discovered this today when looking at the underlying dependency library code. If we had known from the beginning that it uses \r\n, maybe we wouldn't have encountered this issue.

As for this specific problem, personally, I think this line of code may still be meaningful as it adds robustness to the program. However, ultimately it depends on your opinion.

ps:

Some key error locations in the code may still benefit from adding some logs, for example.

dify/api/core/model_runtime/model_providers/openai_api_compatible/llm/llm.py

Lines 437 to 443 in 80e390b

    
           except json.JSONDecodeError as e: 
        
               yield create_final_llm_result_chunk( 
        
                   index=chunk_index + 1, 
        
                   message=AssistantPromptMessage(content=""), 
        
                   finish_reason="Non-JSON encountered." 
        
               ) 
        
               break

Yeuoly

LGTM

bowenliang123 · 2024-04-18T06:24:51Z

Thanks for the explanation. SGTM.

* feat: increase read timeout of OpenAI Compatible API, Ollama, Nvidia LLM (langgenius#3538) * feat: agent log (langgenius#3537) Co-authored-by: jyong <[email protected]> * fix: typo of PublishConfig (langgenius#3540) * fix: workflow delete edge (langgenius#3541) * feat: filter empty content messages in llm node (langgenius#3547) * fix: json-reader-json-output (langgenius#3552) * fix: tool node show output text variable type error (langgenius#3556) * feat: economical index support retrieval testing (langgenius#3563) * optimize question classifier prompt and support keyword hit test (langgenius#3565) * fix event/stream ping (langgenius#3553) * enhance: preload general packages (langgenius#3567) * added claude 3 opus (langgenius#3545) * feat: code (langgenius#3557) * feat: add workflow api in Node.js sdk (langgenius#3584) * Fix: use debounce for switch (langgenius#3585) * fix: json in raw text sometimes changed back to key value in HTTP node (langgenius#3586) * test: add scripts for running tests on api module both locally and CI jobs (langgenius#3497) * add-open-mixtral-8x22b (langgenius#3591) * docs: Update README.md (langgenius#3577) * enhance:speedup xinference embedding & rerank (langgenius#3587) * fix(openai_api_compatible): fixing the error when converting chunk to json (langgenius#3570) * feat: stable diffusion 3 (langgenius#3599) * Feat/enterprise sso (langgenius#3602) * Add mixtral 8x22b (langgenius#3606) * fix: copy invite link has duplicated origin (langgenius#3608) * seucirty: http smuggling (langgenius#3609) * chore: apply ruff rules on tests and app.py (langgenius#3605) * feat: Vision switch functionality is provided on OpenRouter (langgenius#3564) * get dict key indexing_technique in DocumentAddByFileApi (langgenius#3615) Co-authored-by: songqijun <[email protected]> * fix: chat rename (langgenius#3627) * feat: moonshot fc (langgenius#3629) * add-llama3-for-nvidia-api-catalog (langgenius#3631) * content fix to continue (langgenius#3633) Co-authored-by: xiaohan <[email protected]> * Fix error in [Update yaml and py file in Tavily Tool] (langgenius#3465) Co-authored-by: Yeuoly <[email protected]> * feat: add file log (langgenius#3612) Co-authored-by: liuzhenghua-jk <[email protected]> * fix: validate languages (langgenius#3638) * Fix problem with scroll inside chat window (langgenius#3578) * fix: in alembic's offline mode (db migrate with --sql option), skip data operations (langgenius#3533) * fix: workflow_run_id not log_id in workflow api doc (langgenius#3658) * Optimize README_CN (langgenius#3660) * fix: delete tool parameters cache when sync draft workflow for run workflow use new parameter change in draft workflow (langgenius#3637) * python 3.12 support (langgenius#3652) * version to 0.6.4 (langgenius#3670) --------- Co-authored-by: takatost <[email protected]> Co-authored-by: KVOJJJin <[email protected]> Co-authored-by: jyong <[email protected]> Co-authored-by: Bowen Liang <[email protected]> Co-authored-by: zxhlyh <[email protected]> Co-authored-by: Yeuoly <[email protected]> Co-authored-by: Joel <[email protected]> Co-authored-by: Jyong <[email protected]> Co-authored-by: liuzhenghua <[email protected]> Co-authored-by: Siddharth Jain <[email protected]> Co-authored-by: Joshua <[email protected]> Co-authored-by: Matheus Mondaini <[email protected]> Co-authored-by: 呆萌闷油瓶 <[email protected]> Co-authored-by: aniaan <[email protected]> Co-authored-by: Garfield Dai <[email protected]> Co-authored-by: jeessy2 <[email protected]> Co-authored-by: sqj8899 <[email protected]> Co-authored-by: songqijun <[email protected]> Co-authored-by: fuckqqcom <[email protected]> Co-authored-by: xiaohan <[email protected]> Co-authored-by: Richards Tu <[email protected]> Co-authored-by: Yeuoly <[email protected]> Co-authored-by: liuzhenghua-jk <[email protected]> Co-authored-by: YidaHu <[email protected]> Co-authored-by: rmmedia <[email protected]> Co-authored-by: saga.rey <[email protected]> Co-authored-by: xin.gao <[email protected]>

… json (langgenius#3570)

fix(openai_api_compatible): fixing the error when converting chunk to…

01e726b

… JSON

dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. 🐍 python 🐞 bug Something isn't working labels Apr 17, 2024

crazywoola requested a review from Yeuoly April 18, 2024 05:40

Yeuoly approved these changes Apr 18, 2024

View reviewed changes

dosubot Bot added the lgtm This PR has been approved by a maintainer label Apr 18, 2024

Yeuoly merged commit aa6d2e3 into langgenius:main Apr 18, 2024

dengpeng pushed a commit to dengpeng/dify that referenced this pull request Jun 16, 2024

fix(openai_api_compatible): fixing the error when converting chunk to…

6e10add

… json (langgenius#3570)

HuberyHuV1 pushed a commit to HuberyHuV1/dify that referenced this pull request Jul 22, 2024

fix(openai_api_compatible): fixing the error when converting chunk to…

13f7b59

… json (langgenius#3570)

Navigation Menu

Uh oh!

fix(openai_api_compatible): fixing the error when converting chunk to json - #3570

fix(openai_api_compatible): fixing the error when converting chunk to json#3570
Yeuoly merged 1 commit into
langgenius:mainfrom
aniaan:fix/openai_api_compatible

aniaan commented Apr 17, 2024

Uh oh!

bowenliang123 commented Apr 17, 2024 •

edited

Loading

Uh oh!

aniaan commented Apr 18, 2024

Uh oh!

Yeuoly left a comment

Uh oh!

bowenliang123 commented Apr 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

aniaan commented Apr 17, 2024

Description

Type of Change

How Has This Been Tested?

Suggested Checklist:

Uh oh!

bowenliang123 commented Apr 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aniaan commented Apr 18, 2024

Uh oh!

Yeuoly left a comment

Choose a reason for hiding this comment

Uh oh!

bowenliang123 commented Apr 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bowenliang123 commented Apr 17, 2024 •

edited

Loading