octokit / rest.js

GitHub REST API client for JavaScript
https://octokit.github.io/rest.js
MIT License
548 stars 64 forks source link

`octokit.repos.getArchiveLink()` not working in browser #4

Open gr2m opened 6 years ago

gr2m commented 6 years ago

this is a follow up for https://github.com/octokit/rest.js/issues/736 about .repos.getArchiveLink

One way to handle it is to add a new flag like "hasRedirectResponse" in lib/routes.json. If it is set then the method should be HEAD and the method should return with the value of the Location response header. Note that it’s currently not accessible using fetch, the GitHub API needs to return an additional header, see https://stackoverflow.com/a/38975988/206879

Another alternative would be only replace repos.getArchiveLink with repos.getArchive and then add to the documentation that if the user does not want to follow the redirect, they have to set method: 'HEAD' in the request options and then read out the URL from the response’s headers location

sgvictorino commented 6 years ago

@gr2m could you please provide an example of the proposed documentation for repos.getArchive? I'm having trouble getting the response into a file myself, and that's holding up my testing of some aspects of octokit/octokit.js#914.

gr2m commented 6 years ago

Unfortunately this is currently blocked by GitHub’s APIs which doesn’t expose the location header correctly. Once that is resolved this code should work

client.repos.getArchiveLink({
  method: 'HEAD',
  owner: 'octocat',
  repo: 'Hello-World',
  archive_format: 'tarball',
  ref: 'master'
})

  .then(response => {
    console.log(response.headers.location)
  })

Sorry for all the trouble, it’s all a bit more complicated with the JavaScript Octokit. I’ll follow up with the Hubbers on the state of that

If you like you can create a separate issue describing what exactly you are trying to achieve and I’ll help you to find the best workaround for now? Also let me know if your code is run in Node.js or Browser

gr2m commented 5 years ago

It looks like GitHub just exposed the location header, but I still cannot get the location header:

fetch('https://api.github.com/repos/octocat/Hello-World/tarball/master', {method: 'HEAD', redirect: 'manual'})

  .then(response => {
    console.log(response.headers.get('location'))
  })

I’m now pretty sure that it’s not possible to access headers from a redirect response due to security concerns.

However the above code works with node-fetch, so it should work in Node

gr2m commented 5 years ago

what is possible is this:

fetch('https://api.github.com/repos/octocat/Hello-World/tarball/master', {method: 'HEAD'})

  .then(response => {
    console.log(response.url)
  })

But the CORS setting of the codeland.github.com does not allow for that at this point, I contacted the API team

gr2m commented 5 years ago

I got a response from GitHub support that they are looking into updating the CORS setting for codeland.github.com

zeke commented 5 years ago

I just bumped into this today. Here's how I'm working around it for now on a private repo:

const got = require('got')
const { headers } = await got('https://api.github.com/repos/github/some-private-repo/tarball', {
  json: true,
  followRedirect: false,
  headers: {
    accept: 'application/vnd.github.v3+json',
    authorization: `token ${process.env.GITHUB_TOKEN}`
  }
})

console.log(headers.location)
// https://codeload.github.com/github/some-private-repo/legacy.tar.gz/master?token=XXX
gr2m commented 5 years ago

You shouldn’t need a workaround for Node, it looks like there is actually a bug, we should set response.url. Here is a test case: https://runkit.com/gr2m/octokit-rest-js-881/1.0.0 I’ll look into it

gr2m commented 5 years ago

@zeke it now works in Node

https://runkit.com/gr2m/octokit-rest-js-881/1.1.0

cdibbs commented 5 years ago

I got a response from GitHub support that they are looking into updating the CORS setting for codeland.github.com

Any word from them on this? I bumped into this, today, while exploring an idea.

gr2m commented 5 years ago

No update I’m afraid.

What’s your use case? Make sure to contact support and ask them about it. The more people ask about it, the more likely someone will look into it. Reference this issue :)

cdibbs commented 5 years ago

What’s your use case?

I am hoping to implement a simple SPA that would allow an individual user to fetch small repos that the SPA would then manipulate prior to user downloading them.

Make sure to contact support and ask them about it.

Done. Thanks for the suggestion. :-)