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

README "Usage with Octokit" example creates errors, omits info when used with GitHub App #407

Open
Dottenpixel opened this issue Oct 19, 2022 · 3 comments
Labels
Type: Documentation Improvements or additions to documentation

Comments

@Dottenpixel
Copy link

Dottenpixel commented Oct 19, 2022

I'm using the example code in the README verbatim try to use my GitHub App to run a node script in a GitHub Action job. The example under the Usage with Octokit README header is :

const { slug } = await appOctokit.request("GET /user");
                 ^^^^^

SyntaxError: await is only valid in async functions and the top level bodies of modules

After some professional Googling, I found that wrapping the await line in an anonymous async function is the way to go. When I do that I then get this error:

Error: [@octokit/auth-app] installationId option is required for installation authentication.
    at getInstallationAuthentication (/home/runner/work/design-system-internal/design-system-internal/node_modules/@octokit/auth-app/dist-node/index.js:131:11)
    at hook (/home/runner/work/design-system-internal/design-system-internal/node_modules/@octokit/auth-app/dist-node/index.js:359:13)
    at /home/runner/work/design-system-internal/design-system-internal/node_modules/@octokit/plugin-request-log/dist-node/index.js:[18](https://github.com/salesforce-ux/design-system-internal/actions/runs/3285141382/jobs/5411912194#step:8:19):12
    at /home/runner/work/design-system-internal/design-system-internal/node_modules/before-after-hook/lib/register.js:25:15
    at async /home/runner/work/design-system-internal/design-system-internal/gh-app-auth.js:24:30

The README doesn't include any mention of adding an installationId under that header. Can we get the README updated to display a working GitHub App example with Octokit?

@Dottenpixel Dottenpixel changed the title README example creates error, then README Usage with Octokit example creates errors, omits info Oct 19, 2022
@Dottenpixel Dottenpixel changed the title README Usage with Octokit example creates errors, omits info README "Usage with Octokit" example creates errors, omits info Oct 19, 2022
@Dottenpixel Dottenpixel changed the title README "Usage with Octokit" example creates errors, omits info README "Usage with Octokit" example creates errors, omits info when used with GitHub App Oct 20, 2022
@gr2m
Copy link
Contributor

gr2m commented Oct 20, 2022

top-level await is supported since Node.js v14.8. Node 14 is the oldest still maintained node version, so that should no longer be a problem? What is your node version? (node --version)

The README doesn't include any mention of adding an installationId under that header

It's a constructor option that you set as new Octokit({ auth: { installationId }})

Depending on what you are trying to achieve, you probably want to use the App SDK?
https://github.com/octokit/app.js

@Dottenpixel
Copy link
Author

Interesting, I'm well above v14.8. My node version is v16.17.1 coming from actions/setup-node@v3 inside an Actions runner.

I'll take a look at the App SDK. I got where the installationId is used eventually, but others like me coming across that README could be better helped. Maybe my use case isn't typical?

Thanks @gr2m!

@gr2m
Copy link
Contributor

gr2m commented Oct 21, 2022

My node version is v16.17.1 coming from actions/setup-node@v3 inside an Actions runner.

ah sorry my fault, the top-level await is only when using ESM.

It's a trade off. Some libraries do the wrapping in async function, but it creates a lot of noise, so I opted for the simpler approach and hope folks understand how to run them.

I don't know your usecase, but I think using @ocotkit/auth-app directly is not necessary, the differences between app and installation authentication is rather hard to grasp. Using the @octokit/app SDK has better APIs to differentiate between the two.

@wolfy1339 wolfy1339 added the Type: Documentation Improvements or additions to documentation label Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants