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

Allow to inject parameters into webhook handlers #44

Closed
gr2m opened this issue Nov 28, 2018 · 3 comments
Closed

Allow to inject parameters into webhook handlers #44

gr2m opened this issue Nov 28, 2018 · 3 comments
Labels
Type: Feature New feature or request
Projects

Comments

@gr2m
Copy link
Contributor

gr2m commented Nov 28, 2018

For the upcoming octokit we need to be able to inject a pre-authenticated octokit client to the webhooks handler, see the Webhooks section

const webhooks = client.webhooks({secret})
webhooks.on('issue.opened', ({id, name, payload, client}) => {
  return client.rest.issues.createComment({body: 'Hello, World!'})
})
require('http').createServer(webhooks.middleware).listen(3000)

The client parameter in the code snippet above needs to be injected, it is not part of @octokit/webhooks API. We need to be able to run code after a webhook is received, but before it is passed to the webhook handlers.

One approach would be to implement middleware similar to what Express does: https://expressjs.com/en/guide/using-middleware.html. There is a related issue on Probot: probot/probot#598

An alternative would be to leverage the hook API that we already use in @octokit/rest and that will be part of octokit: https://github.com/octokit/rest.js#hooks.

@gr2m gr2m added this to To do in JS via automation Nov 28, 2018
@gr2m gr2m moved this from To do to In progress in JS Nov 28, 2018
@gr2m gr2m self-assigned this Dec 29, 2018
@gr2m
Copy link
Contributor Author

gr2m commented Dec 29, 2018

I will use the before-after-hook like we already do in @octokit/rest. I will allow to pass an optional hook option to the constructor so that the upcoming octokit client can use the same hook instance for both, request and webhook hooks.

const WebhooksApi = require('@octokit/webhooks')
const OctokitRest = require('@octokit/rest')
const webhooks = new WebhooksApi({
  secret: 'mysecret'
})

webhooks.hook.before('webhook', (options) => {
  options.client = new OctokitRest()
  options.client.authenticate({
    type: 'token',
    token: process.env.GITHUB_TOKEN
  })
})

webhook.on('foo.bar', options => {
  // options.client is set now
})

webhooks.receive({id: '123', name: 'foo', payload: {action: 'bar'}})
// logs "Webhook received: foo.bar (id: 123)"

@gr2m
Copy link
Contributor Author

gr2m commented Dec 29, 2018

The current .transform option might actually be good enough for this, it was created for Probot for exactly this reason: https://github.com/probot/probot/pull/335/files#diff-687648a33bac039342470366b890374cR15

@gr2m gr2m moved this from In progress to To do in JS Jan 22, 2019
@gr2m gr2m removed this from To do in JS Sep 4, 2019
@gr2m gr2m added the Type: Feature New feature or request label May 5, 2020
@gr2m gr2m added this to In progress in JS May 5, 2020
@gr2m gr2m removed their assignment May 5, 2020
@gr2m gr2m moved this from In progress to Features in JS May 5, 2020
@gr2m
Copy link
Contributor Author

gr2m commented Feb 4, 2021

The current .transform option might actually be good enough for this

It is.

@gr2m gr2m closed this as completed Feb 4, 2021
JS automation moved this from Features to Done Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New feature or request
Projects
No open projects
JS
  
Done
Development

No branches or pull requests

1 participant