Error after updating Peril

[jlengstorf]

Using Heroku, we followed the steps to pull the latest from danger/peril@master and push to Heroku.

After the update, we're now getting an error:

Aug 15 18:28:42 peril-gatsbyjs app/web.1: info: ## issues.opened on unknown on gatsbyjs/peril-gatsbyjs 
Aug 15 18:28:42 peril-gatsbyjs app/web.1: info:    2 runs needed: org/emptybody.ts, org/labeler.ts 
Aug 15 18:28:43 peril-gatsbyjs heroku/router: at=info method=POST path="/webhook" host=peril-gatsbyjs.herokuapp.com request_id=10aeb0fa-bfd0-40fd-a8db-e5fcbbd66a6e fwd="192.30.253.29" dyno=web.1 connect=0ms service=981ms status=200 bytes=353 protocol=https 
Aug 15 18:28:43 peril-gatsbyjs app/web.1: info: [runner] - Commenting, with results:  
Aug 15 18:28:43 peril-gatsbyjs app/web.1: mds: 0 
Aug 15 18:28:43 peril-gatsbyjs app/web.1: messages: 0 
Aug 15 18:28:43 peril-gatsbyjs app/web.1: warns: 0 
Aug 15 18:28:43 peril-gatsbyjs app/web.1: fails: 0 
Aug 15 18:28:43 peril-gatsbyjs app/web.1: error: Error:  TypeError: Cannot read property 'concat' of undefined 
Aug 15 18:28:43 peril-gatsbyjs app/web.1:     at Executor.<anonymous> (/app/node_modules/danger/distribution/runner/Executor.js:273:46) 

Looking at the danger source, the results.fails property appears to be coming back undefined.

We also might be doing something wrong; our Peril instance recently started triple-firing and I have no idea how or why. Any insights here are much appreciated.

• Repo: https://github.com/gatsbyjs/peril-gatsbyjs
• Triple-tap: https://github.com/gatsbyjs/peril-gatsbyjs/issues/28
• Peril settings: https://github.com/gatsbyjs/peril-gatsbyjs/blob/72dca531d57b801c1c9c2520c0e0ba30d51d35bf/peril.settings.json

Thanks!

[orta]

OK, so we've been having a think about this for a while in https://github.com/ashfurrow/peril-settings/issues/8#issuecomment-412282805

but when https://github.com/danger/peril/pull/352 merges, then if you add SKIP_CHECKS_SUPPORT to a truth string in your env then Peril will skip the checks API stuff completely, which should work around this for now.

[orta]

/cc @ashfurrow

[jlengstorf]

@orta Can you add a little context here? Is our configuration incorrect to require disabling checks, or is it something else?

I've added the following to our Heroku instance:

After restarting, I redelivered an issue creation payload and it still hits the concat error above.

Did I add that env var properly?

[jlengstorf]

For reference, I pulled the latest from master and pushed it up to Heroku this morning before trying this.

[jlengstorf]

I scaled my Heroku web processes down to 0 and back to 1 and now the triple-posting is gone. I have no idea what that was all about.

[jlengstorf]

Ugh. It started happening again after restarting the Heroku instance. I'm pretty lost here. 😢

[ashfurrow]

Hey @jlengstorf – sorry to hear that you're having that problem, it sounds frustrating. That env var looks right, and looking at the code it should work. I'll try to take a look this afternoon. Unfortunately, Orta has been pulled away unexpectedly, so it may be a few days before this can be resolved.

I know that Peril has been integrated into Gatsby's infrastructure and that it's important that it works – if possible, I would consider reverting to your earlier deployed commit. Heroku's dashboard should let you do this through its UI. I'll keep you updated.

[jlengstorf]

I just dumped The env vars and it comes back undefined from process.env. Is there a trick to getting env vars into Peril's env?

I'll try to dig into the source a bit more as well. Gatsby is planning to go all-inclusive on Peril, so I want to make sure it's not a mystery box. 😅

Jason Lengstorf Get #deepinthecreep: Twitter https://twitter.com/jlengstorf · GitHub https://github.com/jlengstorf · LinkedIn https://linkedin.com/in/jlengstorf/ Oversharing at lengstorf.com https://lengstorf.com/

[ashfurrow]

Hmm! That is unexpected. Looking at another place that Peril accesses env vars, it should be as straightforward as you've done in your screenshot:

https://github.com/danger/peril/blob/01a0357ab8139919499a59ee9e04cdffb28612bf/source/db/getDB.ts#L9

Peril/Danger can be a bit tricky to debug. Orta has done some work making it easier, but if you have suggestions or ideas, make sure to drop them into an issue 👍

[ashfurrow]

Hmmm, okay so I've updated my Peril install on Heroku and have set the env var. I'm still seeing the failure involving concat, but I opened a Node REPL on Heroku and the env var is being set correctly:

> const { SKIP_CHECKS_SUPPORT } = process.env
undefined
> SKIP_CHECKS_SUPPORT
'true'
> !SKIP_CHECKS_SUPPORT
false

Which suggests that the code in #352 is at least accessing the environment variables correctly, even if it's not behaving as intended. I'll do some more debugging and get back to you.

[ashfurrow]

I think I have a fix, I'm testing on my Peril instance first and will send a PR once I've verified it works.

[ashfurrow]

Okay, @jlengstorf I've got a PR with a fix up: https://github.com/danger/peril/pull/355 You should be able to pull in that commit immediately and use it. You can optionally remove the environment variable to use the new GitHub checks API (though I've not confirmed that it works, I personally might keep it disabled). /cc @SD10