feat: deprecate onUnhandledRequest and accepting object payloads for for webhooks.verify() and webhooks.verifyAndReceive()

[wolfy1339]

Resolves #784
Resolves #589
See also #775

---

Behavior

Before the change?

• The onUnhandledRequest option was not deprecated
• One could pass an object to the API and due to how Node works, it could change the object slightly so it wouldn't validate

After the change?

• Deprecate the onUnhandledRequest option
• Users will be required to pass a string version of the JSON object

Other information

---

Additional info

Pull request checklist

• Tests for the changes have been added (for bug fixes / features)
• Docs have been reviewed and added / updated if needed (for bug fixes / features)
• Added the appropriate label for the given change

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

• Yes (Please add the Type: Breaking change label)
• No

If Yes, what's the impact:

• N/A

Pull request type

Please add the corresponding label for change this PR introduces:

• Bugfix: Type: Bug
• Feature/model/API additions: Type: Feature
• Updates to docs or samples: Type: Documentation
• Dependencies/code cleanup: Type: Maintenance

---

[wolfy1339]

The deprecations still need to be added to the middleware.
That will be another PR, as it requires modifying the middleware to accept string payloads

[nickfloyd]

LGTM! ❤️ Thanks for getting these issues knocked out!

[github-actions]

🎉 This PR is included in version 10.5.0 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[github-actions]

🎉 This PR is included in version 11.0.0-beta.3 🎉

The release is available on:

• GitHub release
• npm package (@beta dist-tag)

Your semantic-release bot 📦🚀

[AaronDewes]

I know this has been merged, but shouldn't this be a warning instead of an error? It being an error seems to confuse Jest in some tests I run, and a warning would fit here better in my opinion.

[austenstone]

Should this example be updated? How do we do the equivalent of this with just a string?

[wolfy1339]

Simply don't pass the string received into JSON.parse() and the example should work

[austenstone]

I see payload can't be an object.

    this.source = new EventSource(webhookProxyUrl);
    this.source.onmessage = (event) => {
      const webhookEvent = JSON.parse(event.data);
      this.app.webhooks
        .verifyAndReceive({
          id: webhookEvent["x-request-id"],
          name: webhookEvent["x-github-event"],
          payload: JSON.stringify(webhookEvent.body), // <= HERE
          signature: webhookEvent["x-hub-signature"]
        })
        .catch(console.error);
    };

[wolfy1339]

What version of @octokit/webhooks are you using?

What is the exact message you are getting and from where?

I don't see why it would error

[austenstone]

What is the exact message you are getting and from where?

I don't see why it would error

[@octokit/webhooks] Passing a JSON payload object to verifyAndReceive() is deprecated and the functionality will be removed in a future release of @octokit/webhooks

When I pass object instead of string the payload in verifyAndReceive param.

"octokit": "^2.0.15"

App from import { App, createNodeMiddleware } from "octokit";

[wolfy1339]

I cannot reproduce this issue.

The following code works for me

const { App } = require('octokit'); // Octokit.js v2.0.15 and v2.0.16
const EventSource = require('eventsource');

const webhookProxyUrl = "https://smee.io/IrqK0nopGAOc847";
const source = new EventSource(webhookProxyUrl);

const app = new App({
  appId: 95608,
  privateKey: readFileSync('./private-key.pem').toString(),
  webhooks: {
    secret: 'thisismysecret'
  }
});

source.onmessage = (event) => {
  const webhookEvent = JSON.parse(event.data);
  app.webhooks
    .verifyAndReceive({
      id: webhookEvent["x-request-id"],
      name: webhookEvent["x-github-event"],
      signature: webhookEvent["x-hub-signature"],
      payload: JSON.stringify(webhookEvent.body),
    })
    .catch(console.error);
};

[austenstone]

payload: JSON.stringify(webhookEvent.body)

If this isn't a string the warning will appear.

Ex:
payload: webhookEvent.body

[wolfy1339]

That is the intended behaviour