fix: update GraphQL to return correct HTTP response code for file size limit upload errors

[ardatan]

trafficstars

New Pull Request Checklist

• [X ] I am not disclosing a vulnerability.
• [X] I am creating this PR in reference to an issue.

Issue Description

Previously GraphQL Yoga was returning 500 but in the latest version it returns 400 which also not 100% correct. It should be 413 which indicates that user sends a request over the specified limit per HTTP spec. https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/413

https://github.com/dotansimha/graphql-yoga/pull/1270 With the PR above on GraphQL Yoga repo, the behavior is fixed per spec. But Parse Server tests need to be updated since it was expecting the wrong status code (500) which indicates a misleading internal server error that isn't related to the user.

Related issue: https://github.com/parse-community/parse-server/pull/8043#issuecomment-1155742360

Approach

TODOs before merging

• [ ] Add tests
• [ ] Add changes to documentation (guides, repository pages, in-code descriptions)
• [ ] Add security check
• [ ] Add new Parse Error codes to Parse JS SDK
• [x] A changelog entry is created automatically using the pull request title (do not manually add a changelog entry)

[parse-github-assistant[bot]]

Thanks for opening this pull request!

• ❌ Please link an issue that describes the reason for this pull request, otherwise your pull request will be closed. Make sure to write it as Closes: #123 in the PR description, so I can recognize it.

[Moumouls]

Hi @ardatan, parse-server still use npm lockfile v1. So to install the project use nvm use 14 && npm i to avoid a lockfile migration. Then you should have less diffs.

[ardatan]

@Moumouls Sure I will try :) Is there anything I can do so we can get CI running for this PR?

[mtrezza]

@ardatan if a PR is in the works and not ready for review yet you can change its status to "draft"; I've started the CI

[codecov[bot]]

Codecov Report

Patch coverage has no change and project coverage change: -6.91 :warning:

Comparison is base (2ea4e37) 94.18% compared to head (f9049cf) 87.27%.

:exclamation: Current head f9049cf differs from pull request most recent head c85de55. Consider uploading reports for the commit c85de55 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##            alpha    #8082      +/-   ##
==========================================
- Coverage   94.18%   87.27%   -6.91%     
==========================================
  Files         182      182              
  Lines       13712    13712              
==========================================
- Hits        12914    11967     -947     
- Misses        798     1745     +947     

see 24 files with indirect coverage changes

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[ardatan]

Could you run it again @mtrezza ? Thanks!

[mtrezza]

Do you want to test anything specific in this CI that you cannot test locally by running npm test?

[ardatan]

We released a new version now, I updated the PR and ran tests locally it passes. Hopefully it will pass on CI as well. @mtrezza

[ardatan]

@mtrezza There was an issue with Node 12 support. So I updated the PR again.

[mtrezza]

Restarted CI

[Moumouls]

Hi @ardatan thanks, i think we have merge conflicts here, related to the package.lock.

Could you rebase your PR on alpha and then push again (or fix package.lock conflicts) then we should be okay 🚀

failing CI on node 17 is related to a flacky test on apple auth game center

[mtrezza]

I've resolved the conflicts, waiting for CI to pass

[mtrezza]

Not sure how to handle this change of which is essentially a 3rd party-managed API. This is a clear example of why https://github.com/parse-community/parse-server/issues/7979 is so important to move GraphQL into an adapter, out of the core of Parse Server.

On the other hand if the file size limit is an undocumented GraphQL feature so far (or "experimental"), there may not even be a formal issue merging this as a "fix". As @n1ru4l said: "From our perspective, the file upload limit was undocumented and treated as unstable."

Suggested changelog entry for this PR:

fix: update GraphQL to return correct HTTP response code for file size limit upload errors; the HTTP response code changes from 500 to 413; the file size limit mechanism in GraphQL is an undocumented feature, if you are using this feature and parsing the response code make sure to adapt your app code accordingly before upgrading Parse Server.

@n1ru4l please correct me if the "is an undocumented feature" does not accurately describe it

[mtrezza]

What is the state of this PR?

[parse-github-assistant[bot]]

I will reformat the title to use the proper commit message syntax.

[mtrezza]

@n1ru4l, @ardatan, @Moumouls Is this PR still needed? Or can we close it?