octokit / app.js

GitHub Apps toolset for Node.js
MIT License
158 stars 34 forks source link

[BUG]: app.getInstallationOctokit() throws error #455

Closed Udbhav8 closed 5 months ago

Udbhav8 commented 1 year ago

What happened?

import fs from "node:fs"

import { App } from "@octokit/app"
import { type Octokit } from "@octokit/core"
import { Cache } from "@sift/util"

import { type GithubClientCredentials } from "./types"

const githubCache = new Cache<GithubClientCredentials, Promise<Octokit>>()

const createGithubClient = async (credentials: GithubClientCredentials) => {
  credentials.privateKey = fs.readFileSync("private-key.pem", "utf8")
  const app = new App({
    appId: credentials.appId,
    privateKey: credentials.privateKey,
  })

  const installationOctokit = await app.getInstallationOctokit(40990679)
  return installationOctokit
}

const getGithubClient = (githubClientCredentials: GithubClientCredentials) =>
  githubCache.getAndCache(githubClientCredentials, createGithubClient)

export default getGithubClient

The app.getInstallationOctokit() function does not behave as expected and throws the error message given below. It looks like the error originates from the library universal-github-app-jwt library which is using the browser version of node instead.

Versions

octokit/app: ^14.0.0 node version : v18.17.0

Relevant log output

`[14:42:17.311] ERROR (68072): Error while handling interaction 
    err: {
      "type": "ReferenceError",
      "message": "crypto is not defined",
      "stack":
          ReferenceError: crypto is not defined
              at getToken (/Users/udbhavagarwal/Documents/GitHub/sift/node_modules/@octokit/auth-app/node_modules/universal-github-app-jwt/dist-src/get-token-browser.js:13:25)
              at githubAppJwt (/Users/udbhavagarwal/Documents/GitHub/sift/node_modules/@octokit/auth-app/node_modules/universal-github-app-jwt/dist-src/index.js:14:25)
              at getAppAuthentication (/Users/udbhavagarwal/Documents/GitHub/sift/node_modules/@octokit/auth-app/dist-node/index.js:53:73)
              at getInstallationAuthentication (/Users/udbhavagarwal/Documents/GitHub/sift/node_modules/@octokit/auth-app/dist-node/index.js:227:35)
              at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
              at hook (/Users/udbhavagarwal/Documents/GitHub/sift/node_modules/@octokit/auth-app/dist-node/index.js:393:32)
              at _GithubMessageHandlerImpl.forward (/Users/udbhavagarwal/Documents/GitHub/sift/apps/ticketing/src/lib/github/github-message-handler.ts:66:21)
              at broadcastMessage (/Users/udbhavagarwal/Documents/GitHub/sift/apps/ticketing/src/lib/constructs/broadcast-message.ts:67:5)
              at Object.handle (/Users/udbhavagarwal/Documents/GitHub/sift/apps/ticketing/src/lib/slack/interactions/send-to-sift-shortcut.ts:48:5)`

Code of Conduct

github-actions[bot] commented 1 year 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! 🚀

wolfy1339 commented 1 year ago

Are you using ES Modules by chance? Can you share your tsconfig.json? Are you using a bundler?

arturenault commented 1 year ago

Hi - I'm working on this project with @Udbhav8.

wolfy1339 commented 1 year ago

What I think happened is that SST is using the wrong package files, probably the dist-browser due to how the package.json defines the module field.

Is there a way you can force it to use dist-src directly instead of relying on what is defined in the package.json?

The build system it uses is no longer maintained, but the package was rewritten to ESM in v2 so it no longer uses that build system.

We're stuck with version 1 of that package unfortunately.

I don't know if it's worth changing the build system since v1 is only in maintenance releases

wolfy1339 commented 1 year ago

@Udbhav8 @arturenault Do you have any updates? Is this still an issue

wolfy1339 commented 5 months ago

We switched to ESM natively and removed CJS. The universal-github-app-jwt library was updated as well and it should fix this issue.

Closing since no response