Only sets req.raw.body if body defined, aligning behaviour with node:http

[didley]

Currently Middie's undefined req.body handling does not align with node:http and express. Their behaviour, If the body has not been parsed with a body parser and is undefined, the body key will not be present within the request. Maddie includes the body key with an undefined value.

I've created this PR as the library @octokit/webhooks does not currently work with Middie[0] because of this issue. I also feel it would be beneficial to align the behaviour where possible.

Apologies, I haven't run the benchmark script, I don't see it available on main. Have I missed something?

[0] octokit/webhooks.js#1009

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[mcollina]

lgtm

[mcollina]

Can you also skip Mac on v14 and v16?

[didley]

Thanks for the review @mcollina, I've made the v8 change. Regarding skipping Mac on v14 and v16, do I just need to upgrade the fastify workflow version? Comparing the difference between previous CI jobs I'm assuming that's why it was skipped in this PR[0]

[0] https://github.com/fastify/middie/pull/199/files#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03f

[Uzlopak]

Please revert this.

Suggested change

	uses: fastify/workflows/.github/workflows/plugins-ci.yml@v4.1.0
	uses: fastify/workflows/.github/workflows/plugins-ci.yml@v3

[didley]

Thanks Aras, that's now been reverted. This allowed the previous CI run to pass without running on Mac v14 and v16, I'm expecting it will fail again as CI will run again on Mac for these versions.

[didley]

It's wild out of anyone to review this to be you @Uzlopak, this relates to a change you made in the @octokit/webhooks library, in the issue I linked above[0] that I've also created an issue and PR there for.

[0] octokit/webhooks.js#1009

[gurgunday]

lgtm

I suppose this is a bug fix and does not require a major bump?

[didley]

Thanks @gurgunday, yes I would assume it would just be a minor bump.

[mcollina]

lgtm

[didley]

I've just tightened the conditional from if(req.body) to if(req.body !== undefined) as this issue only affects the undefined case. '' and null would indicate parsing of the body.

[didley]

Is there anything else required to get this merged?

From my understanding CI will fail unless I include the next branch CI config (Fastify workflow v4.1.0), if we don't want to include that change within this PR and require CI to pass I could base this PR to the next branch instead?

[gurgunday]

It should be good now, sorry for the intervention, had to trigger the ci

[gurgunday]

lgtm

[didley]

Excellent! Thank you @gurgunday. I'm ready for it to be merged from my side.

[gurgunday]

I'll release a new version

[didley]

Amazing! Thank you all for your assistance with this and your contributions to OSS