fix: determine webhook path for createNodeMiddleware() via env variable

[Uzlopak]

Fixes #1878

When using probot 13 beta i realized that it was totally unintuitive that setting the WEBHOOK_PATH had no effect on our bots.

[Uzlopak]

I prefer the singular form WEBHOOK_PATH.

webhooksPath comes from the @probot/webhooks package.

Just for some background:
We use probot for our bots in combination with google cloud functions. I upgraded to probot 13 and needed to set the webhook path. So i set it as environment variable WEBHOOK_PATH but the trigger was still /api/github/webhooks.
So I realized it is not picking up the path from the env variable.

So we used webhooksPath option. The PR makes it possible that probot picks up the webhooks path from the env variable.

[gr2m]

I think this pull request does more than necessary. I agree with the first test but the 2nd test is odd, all environment variables should be set explicitly as the env argument in createProbot(), no other process.env.* variables should matter, it would result in unforeseen side effects

[Uzlopak]

@gr2m

Implemented as requested. ;)

[gr2m]

I'm still a bit unsure about exposing probot.webhookPath. The path on with webhook requests are received does not logically belong to the probot instance, it belongs to whatever mechanism is used to receive requests, such as createNodeMiddleware or the express server.

Sorry for making this hard, I try to avoid a future breaking change. Once we introduce probot.webhookPath we cannot remove it. I think we should try to solve this in a way that does not require any changes to to create-probot.ts

[gr2m]

if possible, can you avoid force-pushing to pull requests once reviews started? It makes it harder to keep track of progress. PRs can get messy as they need be, we will squash them at the end anyway

[Uzlopak]

Sry regarding forced pushing. Kind of bad habit from creating PRs for nodejs.

When I am at home I will check again. I just want to be able to overwrite the webhookPath via env variable when using createNodeMiddleware.

[gr2m]

I just want to be able to overwrite the webhookPath via env variable when using createNodeMiddleware.

maybe we should move reading that env variable into createNodeMiddleware?

[Uzlopak]

@gr2m

I really invested now multiple hours on this, but your request is contradictive.

createProbot has the env option, where I need to pass WEBHOOK_PATH. So I would expect that that env would handle WEBHOOK_PATH properly. So it has to expose the knowledge somehow.

The only other solution I can propose is to make the state attribute of probot public and make it start with _ to indicate it is internal api and the api contract is not ensured.

[gr2m]

okay I'm sorry I rather not have you frustrated, I don't mind getting this merged, we can revise in the future if needed, but for now lets ship it

[github-actions]

🎉 This PR is included in version 13.0.0-beta.7 🎉

The release is available on:

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

Your semantic-release bot 📦🚀