Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Getting wrong danger object when Peril runs a Dangerfile #431

Open
alephao opened this issue Apr 15, 2019 · 10 comments
Open

Getting wrong danger object when Peril runs a Dangerfile #431

alephao opened this issue Apr 15, 2019 · 10 comments

Comments

@alephao
Copy link

alephao commented Apr 15, 2019

Hello, I've been trying to set up Peril locally by following the docs. Everything works fine until I try to run a Dangerfile.

I cloned the danger/peril-settings and did some small changes to the settings.json file, leaving only the markAsMergeOnGreen.ts and mergeOnGreen.ts.

When I get a github hook for a issue_comment, Peril tries to run the markAsMergeOnGreen.ts that I specified on my settings.json, but the danger object is not the DangerDSL, which is what I'm expecting.

I added a console.log(danger) on the dangerfile, and found out that the object was actually a IssueCommentEvent payload as I show in the logs below.

Not sure if that's a bug or if I'm doing something very wrong. Can someone give me a light?

Thank you!

Logs:

yarn run v1.15.2
$ node out/index.js
info: 
info: ☢️  Starting up Peril
info: ⅹ Papertrail
info: ⅹ Clustering
info: ✓ JSON Db at lalainc/[email protected]
info: ✓ Server:
info:   - Local: http://localhost:5000
[winston] Unknown logger level: router
info: 
info: ## issue_comment.created on unknown on LalaInc/coindar-ios
info:    1 run needed: lalainc/peril-settings@org/markAsMergeOnGreen.ts
@babel/polyfill is loaded more than once on this page. This is probably not desirable/intended and may have consequences if different versions of the polyfills are applied sequentially. If you do need to load the polyfill more than once, use @babel/polyfill/noConflict instead to bypass the warning.
-- DANGERR ---------------
{ github:
   { action: 'created',
     issue:
      { ... },
     comment:
      { ... },
     repository:
      { ... },
     organization:
      { ... },
     sender:
      { ... },
     installation:
      { ... } } }
TypeError: Cannot read property 'orgs' of undefined
    at Object.exports.default (/path/to/peril/api/danger-0.wfe3bdnkd99.ts:33:19)
    at Object.<anonymous> (/path/to/peril/api/node_modules/danger/distribution/runner/runners/inline.js:147:60)
    at step (/path/to/peril/api/node_modules/danger/distribution/runner/runners/inline.js:32:23)
    at Object.next (/path/to/peril/api/node_modules/danger/distribution/runner/runners/inline.js:13:53)
    at /path/to/peril/api/node_modules/danger/distribution/runner/runners/inline.js:7:71
    at new Promise (<anonymous>)
    at __awaiter (/path/to/peril/api/node_modules/danger/distribution/runner/runners/inline.js:3:12)
    at Object.exports.runDangerfileEnvironment (/path/to/peril/api/node_modules/danger/distribution/runner/runners/inline.js:105:136)
    at runDangerAgainstFileInline (/path/to/peril/api/out/danger/danger_runner.js:49:37)
    at process._tickCallback (internal/process/next_tick.js:68:7)
Sender does not have permission to merge
@orta
Copy link
Member

orta commented Apr 15, 2019

For non-PR events you don't get the DangerDSL (because the DSL is built for PRs (and while some issues are PRs, there are many which aren't))

In those cases you get a lot of the utils options but nothing specific to a PR etc, but I mostly use danger.github.api

I should probably find a way to document what's available at some point - yep

@alephao
Copy link
Author

alephao commented Apr 15, 2019

Hmm, so danger.github.api should be available?

Because what I get on danger.github is just the IssueCommentEvent payload, I don't really have any of the utils available, there is no danger.github.api, so Peril complains that Cannot read property 'orgs' of undefined

@orta
Copy link
Member

orta commented Apr 15, 2019

Yeah, that sounds like a weird bug - we have lots of code which relies on danger.github.api from events like this

https://github.com/artsy/peril-settings/blob/master/org/markAsMergeOnGreen.ts#L5-L8

@alephao
Copy link
Author

alephao commented Apr 15, 2019

Yea, I was trying to use markAsMergeOnGreen.

https://github.com/LalaInc/peril-settings/blob/4028ba0f2fdae8f892f191b9b477f63d9214a999/org/markAsMergeOnGreen.ts#L18

This bug happens consistently to me when running locally, not sure what could be causing it

@orta
Copy link
Member

orta commented Apr 15, 2019

Maybe try adding some logs around here: https://github.com/danger/peril/blob/master/api/source/runner/run.ts#L83-L90

I know in old versions of Peril I used to put the JSON payload inside danger.github but when I added support for adding it into the default function args I removed it. It should always be passing a wrapped version of the GitHub object. Interesting

@adam-bratin
Copy link

I am able to reproduce. I think I have root caused the issue:

let githubAPI = null as GitHubAPI | null
if (supportsGithubCommentAPIs && settings.commentableID && settings.repoName) {
githubAPI = githubAPIForCommentable(token, settings.repoName, settings.commentableID)
}

When an event comes in that is not commentable the githubAPI is not created. This in turn means that the api and util objects do not get create for the dangerDSL that gets exposed to the inline dangerfile. That causes the error that repos cannot be called on undefined object.

@alephao
Copy link
Author

alephao commented Apr 30, 2019

@orta I added some logs to this function, and It doesn't seem like this function is being called at all 🤔

@adam-bratin, I looked into it and supportsGithubCommentAPIs was false because the run event was an issue_comment, which returns RunFeedback.silent here:

export const feedbackTypeForEvent = (event: string): RunFeedback => {
if (event === "pull_request" || event === "issues" || event === "issue") {
return RunFeedback.commentable
}
return RunFeedback.silent
}

But not sure if that's the issue. I tried forcing it to return a RunFeedback.commentable, and it seems like this githubAPI is not the same object I'm looking for ☹️

@orta
Copy link
Member

orta commented Apr 30, 2019

(Just letting you folks I'm not ignoring you, this is my last week at Artsy and I don't really have any spare time to think about maintaining OSS stuff ATM)

FWIW, the GitHubAPI object (from DangerJS) vs the OctoKit API are different things - I'd expect the GitHubAPI to be missing on any non-issue event because it needs to have a commentable thing to work with.

I would hope this shouldn't affect the creation of ta GitHubDSL object- but it could be that, yep

@adam-bratin
Copy link

Hey @orta no problem real life happens. Just to give you some more context:
It looks like the logic follows as this:

  • github_runner runEverything calls
//where req.body is the payload of the event and becomes dangerDSL in runEventRun
await runEventRun(eventName, eventRuns, settings, token, req.body)
  • RunEventRun calls runDangerForInstallation with
//(remember dangerDSL is still event details)
{ dsl: { github: dangerDSL } 
  • Then runDangerForInstallation calls runDangerAgainstFileInline passing payload which is
//(dangerDSL still event details)
{ dsl: { github: dangerDSL }, webhook: dangerDSL }
  • now runDangerAgainstFileInline calls contextForDanger which returns back
//danger is still event details
{
    schedule,
    fail,
    warn,
    message,
    markdown,
    results,
    danger: dsl,
    peril: {} as any,
  } 
  • Then createDangerfileRuntimeEnvironment is called which just returns back the above object as dangerRuntimeEnv
  • finally this is used to call runDangerfileEnvironment

This is the overall path of how danger.github.api is undefined

@adam-bratin
Copy link

@orta any update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants