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

bogus http status code for payload too large error #5087

Closed
akosyakov opened this issue Aug 6, 2021 · 3 comments · Fixed by #5329
Closed

bogus http status code for payload too large error #5087

akosyakov opened this issue Aug 6, 2021 · 3 comments · Fixed by #5329

Comments

@akosyakov
Copy link
Member

akosyakov commented Aug 6, 2021

Bug description

I'm hitting the payload too large error for UI state, but the server does not return a proper error code, but internal server error:
Screenshot 2021-08-06 at 10 51 01

We should fix the error code and increase allowed payload size of UI state. The proper status code is 413: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/413#:~:text=The%20HTTP%20413%20Payload%20Too,a%20Retry%2DAfter%20header%20field.

Steps to reproduce

See above.

Expected behavior

No response

Example repository

No response

Anything else?

No response

@csweichel
Copy link
Contributor

/schedule

@aledbf
Copy link
Member

aledbf commented Aug 13, 2021

I'm hitting the payload too large error for UI state,

@akosyakov how I can trigger this error?

@akosyakov
Copy link
Member Author

You should be able to reduce content limit here to reproduce:

router.post('/v1/resource/:resource', bodyParser.text({
limit: config?.contentLimit || defaultContentLimit
}), async (req, res) => {

@jeanp413 pointed out that we return 500 here:

if (!this.isAnsweredRequest(req, response)) {
response.status(500).send({ error: msg });
}
Maybe it is a reason. VS Code frontend expects to see error codes as they returned by code-sync endpoint: https://github.com/microsoft/vscode/blob/ee1655a82ebdfd38bf8792088a6602c69f7bbd94/src/vs/platform/userDataSync/common/userDataSyncStoreService.ts#L476-L512 We should not convert them to 500 anywhere.

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

Successfully merging a pull request may close this issue.

6 participants