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

Elaborate on hash and error handling #36

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@Daniel15
Copy link

commented Jun 7, 2019

Some updates based on my experience with implementing this on the client-side.

  • The hash must be an SHA256 hash. Some implementations just treat the hash as an opaque identifier, but Apollo Server actually validates that it's the right SHA256 hash. This spec should err on the side of being too explicit rather than not explicit enough. It would be good to use a faster non-cryptographic hash (like xxHash), but for now this needs to use SHA256.
  • The PersistedQueryNotFound error is returned with a HTTP 200 status code. Closes #35
@meteor-bot

This comment has been minimized.

Copy link

commented Jun 7, 2019

@Daniel15: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

@codecov-io

This comment has been minimized.

Copy link

commented Jun 7, 2019

Codecov Report

Merging #36 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #36   +/-   ##
=======================================
  Coverage   98.76%   98.76%           
=======================================
  Files           2        2           
  Lines          81       81           
  Branches       23       22    -1     
=======================================
  Hits           80       80           
  Misses          1        1

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 974edc3...2afd58c. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.