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

fix: bump @octokit/webhooks-methods to ^2.0.0 #587

Merged
merged 5 commits into from
Jun 18, 2021

Conversation

gr2m
Copy link
Contributor

@gr2m gr2m commented Jun 17, 2021

@gr2m gr2m added the Type: Bug Something isn't working as documented, or is being fixed label Jun 17, 2021
@gr2m gr2m changed the title fix: bump @octokit/webhooks-methods to ^2.0.0' fix: bump @octokit/webhooks-methods to ^2.0.0 Jun 17, 2021
@wolfy1339 wolfy1339 merged commit 3344b9f into master Jun 18, 2021
@wolfy1339 wolfy1339 deleted the @octokit/[email protected] branch June 18, 2021 04:02
@github-actions
Copy link
Contributor

🎉 This PR is included in version 9.8.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@rr-codes
Copy link

@wolfy1339 This is a breaking change, and led to all my GitHub payloads breaking. This is because @octokit/webhooks-methods had a breaking change requiring the payloads to be string instead of JSON objects, both of which worked fine until this PR, and now only string works.

@rr-codes
Copy link

for context, I am using Express, and had this middleware:

app.use((req, res, next) => {
  if (req.originalUrl.includes('/stripe/webhook')) {
    bodyParser.raw({ type: 'application/json' })(req, res, next);
  } else {
    bodyParser.json()(req, res, next);
  }
});

However, with this PR, I had to change it to

app.use((req, res, next) => {
  if (req.originalUrl.includes('/stripe/webhook')) {
    bodyParser.raw({ type: 'application/json' })(req, res, next);
  } else if (req.originalUrl.includes('/webhooks/github')) {
    bodyParser.text({ type: 'application/json' })(req, res, next);
  } else {
    bodyParser.json()(req, res, next);
  }
});

@wolfy1339
Copy link
Member

It was a breaking change in the @octokit/webhools-methods package, we added a compatibility layer to this package in the meantime until the change is implemented in this package as well.

@rr-codes
Copy link

@wolfy1339 Ya I saw the changes, but the compatibility layer didn't seem to have any effect

@wolfy1339
Copy link
Member

Could you open a new issue with all the details, and we can start looking from there

@rr-codes
Copy link

sure, will do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented, or is being fixed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants