fix: explicitly check that `request.body` is a string

didley commented 5 months ago

Resolves #1009

Before the change?

I am reverting a conditional that changes the handling of request.body that was made within v12.0.10. This broke webhooks for libraries that define the request body key, if body is undefined such as @fastify/middie rather than not including the body property like node:http and express.

After the change?

getPayload() will resolve with a string rather than the undefined if the request body key is defined but has a value undefined.

Pull request checklist

[x] Tests for the changes have been added (for bug fixes / features)
[x] Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

[ ] Yes
[x] No

UPDATE: This may be being fixed upstream within @fastify/middie, if attempting to reproduce this bug please use a Middie <=8.3.0. I think the @octokit/webhooks patch should still be completed as this could be quite difficult to diagnose if the user is on a <=8.3.0 of Middie or if they have constraints for upgrading it.

github-actions[bot] commented 5 months 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 5 months ago

Is it possible to test that this won't break in a future release

didley commented 5 months ago

@wolfy1339 I have added a unit test covering this case. Is there anything else you had in mind to ensue this won't break a future release?

github-actions[bot] commented 5 months ago

:tada: This PR is included in version 13.2.7 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

didley commented 5 months ago

Thank you @G-Rath and everyone else involved!

octokit / webhooks.js