mergeability / mergeable

🤖 All the missing GitHub automation 🙂 🙌
https://mergeable.us
GNU Affero General Public License v3.0
683 stars 116 forks source link

HttpError is crashing the app when it requests to merge a draft PR #731

Open abid-mujtaba opened 10 months ago

abid-mujtaba commented 10 months ago

The app makes a request to the Github (enterprise) API using 'user-agent': 'probot/12.3.3 octokit-core.js/3.6.0 Node.js/18.18.2 (linux; x64)' the merge request is rebuffed because the PR is still a draft. This causes the app to crash:

ERROR (GithubAPI/pulls.merge):
    logType: "unknown_github_api_error"
    eventId: "7b498920-ab0f-11ee-98c8-465914021399"
    repo: "<redacted>"
    event: "pull_request.labeled"
    callFn: "pulls.merge"
    errors: "HttpError: Pull Request is still a draft"
/layers/paketo-buildpacks_npm-install/launch-modules/node_modules/@octokit/request/dist-node/index.js:86
      const error = new requestError.RequestError(toErrorMessage(data), status, {
                    ^
RequestError [HttpError]: Pull Request is still a draft
    at /layers/paketo-buildpacks_npm-install/launch-modules/node_modules/@octokit/request/dist-node/index.js:86:21
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async sendRequestWithRetries (/layers/paketo-buildpacks_npm-install/launch-modules/node_modules/@octokit/auth-app/dist-node/index.js:398:12)
    at async Job.doExecute (/layers/paketo-buildpacks_npm-install/launch-modules/node_modules/bottleneck/light.js:405:18) {
  status: 405,

Does anyone have any idea what might be causing this?

shine2lay commented 10 months ago

at first glance, it seems like the bot is trying to create a merge action on a PR that is still a draft. adding a check in Merge action to skip on PR that are draft might work

abid-mujtaba commented 10 months ago

Indeed. I am wondering, however, if the app crashing here is expected behavior?

shine2lay commented 10 months ago

it's is not the desirable behavior, this is most likely caused by changes to github api over the year that the code is not built to handle.

abid-mujtaba commented 10 months ago

Thanks. Let me start investigating that.

abid-mujtaba commented 10 months ago

This started to happen when I switched the app's underlying node from v14 to v18. I think the app has started crashing on these errors because in v15 the default behavior for unhandled promise rejection was switched to crashing the app.

To address this I added the following to the start of my index.js:

process.on('unhandledRejection', (reason, promise) => {
  console.log('WARNING: Ignoring unhandled promise rejection at:', promise, 'reason:', reason);
});

Do we want to add this for everyone, perhaps controlled by an IGNORE_UNHANDLED_PROMISE_REJECTION env var?