octokit / octokit.js

The all-batteries-included GitHub SDK for Browsers, Node.js, and Deno.
MIT License
6.86k stars 1k forks source link

[BUG]: Octokit doesn't seem to support conditional requests using ETags #2563

Open JWilson45 opened 9 months ago

JWilson45 commented 9 months ago

What happened?

I have been trying to increate the efficiency our app has with its GitHub rate limit by using ETags through octokit. I have a GitHub app and am using the org installation authentication, when a request is sent with the header If-none-match: "etag" we get a 200 back every time. However, if I perform the same request using curl or postman, I will get a 304 Not Modified which is what I expect.

Am I using Etags wrong in the octokit request or does octokit not support the use of Etags. I am using "@octokit/app": "^13.1.8".

An example of the request follows.

Versions

"@octokit/app": "^13.1.8",

Node 18.18.2

Relevant log output

const request = 'GET /repos/{owner}/{repo}/issues';

const options = {
    owner,
    repo,
    page,
    state,
    per_page: perPage,
    headers: {
      'X-GitHub-Api-Version': X_GITHUB_HEADER,
      "If-None-Match": '<etagvalue>'
    },
  };

const resp = await octokit.request(request, options);

Code of Conduct

github-actions[bot] commented 9 months ago

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

gr2m commented 9 months ago

I can assure it does work, but if I recall correctly, you need to include the " quotes in the If-None-Match header value, see https://docs.github.com/en/rest/overview/resources-in-the-rest-api?apiVersion=2022-11-28#conditional-requests

$ curl -I https://api.github.com/user -H 'If-None-Match: "644b5b0155e6404a9cc4bd9d8b1ae730"'
> HTTP/2 304
> Cache-Control: private, max-age=60
> ETag: "644b5b0155e6404a9cc4bd9d8b1ae730"
> Last-Modified: Thu, 05 Jul 2012 15:31:30 GMT
> Vary: Accept, Authorization, Cookie
> x-ratelimit-limit: 5000
> x-ratelimit-remaining: 4996
> x-ratelimit-reset: 1372700873
JWilson45 commented 9 months ago

Hi, thank you for your response!

I am able to get this code to work using a curl request while including the double quotes, doing this returns an expected 304. However, the issue is with Octokit. I am unable to get a 304 response when providing the If-None-Match header with the etag value. Could you show me an example of how to make this request using Octokit.js?

gr2m commented 9 months ago

There you go: https://runkit.com/gr2m/octokit-octokit-js-2563/1.0.0