Add propertyExists() guardian

[gomorizsolt]

This PR adds a propertyExists() function to ensure a property on a target object.

[thisismydesign]

Would this fail the job? Right now it's failing every time too which I want to avoid. I assume there's a happy path that wasn't covered.

[gomorizsolt]

Right now it's failing every time too which I want to avoid.

With what the job is failing? As for the question, returning with Promise.reject() would definitely fail the job.

[gomorizsolt]

We can replace these return statements with Promise.resolve() but this way the job wouldn't fail and therefore we wouldn't be notified if there's a missing property on an object.

[gomorizsolt]

Tweaked the code a little bit: e308886

Given that we're not aware of the exact reason why certain properties are missing on the responses, I'd let the job fail with meaningful error messages. This way we can trace back the errors and act in response to them.

[thisismydesign]

What's the plan with this? We shouldn't release another broken version. We should reproduce and fix the issue.

[gomorizsolt]

What's the plan with this? We shouldn't release another broken version. We should reproduce and fix the issue.

Alright, makes sense. Then I'll need some info as I can't reproduce the issue. Created an instance of the bot there's no error in my case.

Since you also reported the error, could you take a look at your notifications? Isn't there e.g. a system-level notification which doesn't belong to a user?

#14 (comment)

This comment is left unanswered.

[thisismydesign]

Right now it's failing every time too which I want to avoid.

With what the job is failing? As for the question, returning with Promise.reject() would definitely fail the job.

I meant currently the job is failing, but the goal is not to have it fail.

Since you also reported the error, could you take a look at your notifications? Isn't there e.g. a system-level notification that doesn't belong to a user?

Started failing on July 1st 2020 12:04am with this: mbrn/material-table#1592.
The initial issue was fixed but the job kept failing - I'd assume it was the same notification causing it.

Stopped failing July 8th 2020 6:05pm - because the notification is now more than 7 days old. If you extend the 7 days I expect you can reproduce the issue.

[gomorizsolt]

Thanks for this info. This way the error is reproducible but in my case the job fails because of a release notification. I've inspected the response and it seems there's no user property on data but author in the case of releases.

I'd propose adding the following:

const releaseNotification = data.author && !data.user;

if (releaseNotification) {
   // terminate the promise
}

I'd assume it was the same notification causing it.

Yea, it's most likely because of that. But it's interesting because if the comments are listed for that issue specifically then each object contains the user property (even the automated bot message):

{
   "url":"https://api.github.com/repos/mbrn/material-table/issues/comments/652067096",
   "html_url":"https://github.com/mbrn/material-table/issues/1592#issuecomment-652067096",
   "issue_url":"https://api.github.com/repos/mbrn/material-table/issues/1592",
   "id":652067096,
   "node_id":"MDEyOklzc3VlQ29tbWVudDY1MjA2NzA5Ng==",
   "user":{
      "login":"stale[bot]",
      "id":26384082,
      "node_id":"MDM6Qm90MjYzODQwODI=",
      "avatar_url":"https://avatars3.githubusercontent.com/in/1724?v=4",
      "gravatar_id":"",
      "url":"https://api.github.com/users/stale%5Bbot%5D",
      "html_url":"https://github.com/apps/stale",
      "followers_url":"https://api.github.com/users/stale%5Bbot%5D/followers",
      "following_url":"https://api.github.com/users/stale%5Bbot%5D/following{/other_user}",
      "gists_url":"https://api.github.com/users/stale%5Bbot%5D/gists{/gist_id}",
      "starred_url":"https://api.github.com/users/stale%5Bbot%5D/starred{/owner}{/repo}",
      "subscriptions_url":"https://api.github.com/users/stale%5Bbot%5D/subscriptions",
      "organizations_url":"https://api.github.com/users/stale%5Bbot%5D/orgs",
      "repos_url":"https://api.github.com/users/stale%5Bbot%5D/repos",
      "events_url":"https://api.github.com/users/stale%5Bbot%5D/events{/privacy}",
      "received_events_url":"https://api.github.com/users/stale%5Bbot%5D/received_events",
      "type":"Bot",
      "site_admin":false
   },
   "created_at":"2020-06-30T21:56:02Z",
   "updated_at":"2020-06-30T21:56:02Z",
   "author_association":"NONE",
   "body":"This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. You can reopen it if it required.\n"
}

I'm therefore not entirely sure if that's where the issue stems from. Didn't you receive a release notification on 2020-06-30?

I'm open to discussing this.

[gomorizsolt]

As a side note, the user object above has a type property which we can make use of to detect bots.

[thisismydesign]

I like the proposal. I've received a release notification for fresh-bot in fact :D And the time matches, so yes that could've caused the issue.

I also like the type field proposal, could you create an issue for that please?

[gomorizsolt]

I also like the type field proposal, could you create an issue for that please?

Yea, sure: #17

[gomorizsolt]

Closed in favour of #18.