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

Better HTTP Status Code for Empty Requests #365

Closed
2 of 4 tasks
mbbyn opened this issue Aug 1, 2024 · 5 comments · Fixed by #413
Closed
2 of 4 tasks

Better HTTP Status Code for Empty Requests #365

mbbyn opened this issue Aug 1, 2024 · 5 comments · Fixed by #413

Comments

@mbbyn
Copy link

mbbyn commented Aug 1, 2024

System Info

(I don't think it's necessary)

Information

  • Docker
  • The CLI directly

Tasks

  • An officially supported command
  • My own modifications

Reproduction

image

Expected behavior

HTTP status 400 is more appropriate IMHO. The 413 status code might lead to misinterpretation. For example, when integrated with Dify, dify translates the 413 to Payload too large, but there is not payload to begin with.

@mbbyn
Copy link
Author

mbbyn commented Aug 1, 2024

Related langgenius/dify#6807

@OlivierDehaene
Copy link
Member

The 413 status code is causing red-herring, wild goose-chase, witch-hunting, etc.

???

@mbbyn
Copy link
Author

mbbyn commented Sep 17, 2024

Hey @OlivierDehaene, could you please elaborate as to what's the issue with the description? I hope it's nothing offensive, as I was just trying to describe it in a light-hearted manner. If that's not the interpretation, I apologize and can edit it to be more formal.

@eero-t
Copy link

eero-t commented Sep 18, 2024

@OlivierDehaene The returned 413 error for too large requests: https://en.wikipedia.org/wiki/List_of_HTTP_status_codes#4xx_client_errors

Besides it being incorrect one for empty requests, it makes finding the actual issue harder (e.g. if one has large cluster with large number of requests flying around, and that error code only bubbles up to some higher level report).

@OlivierDehaene
Copy link
Member

OlivierDehaene commented Sep 19, 2024

@mbbyn, well it's a strange way to introduce yourself to a complete stranger.
@eero-t, I guess? But at the same time the payload of the error was inputs cannot be empty which makes it pretty obvious to the caller what they need to check...

In any case, #413 uses a 400 instead.

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

Successfully merging a pull request may close this issue.

3 participants