Pass down error log

[shkhaliq]

trafficstars

When the scan step fails due to an error, The error logs of periphery should spill through in the danger log so it's easy to review. Currently it just returns the exit code and the log exits out

[2022-11-29T04:29:20.581Z] [20:29:20]: --- Step: danger ---

[2022-11-29T04:29:20.581Z] [20:29:20]: --------------------

[2022-11-29T04:29:20.581Z] [20:29:20]: $ bundle exec danger --danger_id=periphery_result --dangerfile=PeripheryDangerfile

[2022-11-29T04:30:20.640Z] [20:30:20]: ▸ bundler: failed to load command: danger (/Users/test/vendor/bundle/ruby/2.7.0/bin/danger)

[2022-11-29T04:30:20.751Z] [20:30:20]: ▸ /Users/test/vendor/bundle/ruby/2.7.0/gems/danger-periphery-0.2.2/lib/periphery/runner.rb:16:in `scan': \e[31m (Danger::DSLError)

[2022-11-29T04:30:20.751Z] [20:30:20]: ▸ [!] Invalid `PeripheryDangerfile` file: error: ["/Users/test/.swift_tools/tools/periphery/2.10.0/periphery", "scan", "--config", ".periphery.yml", "--skip-build", "--index-store-path", "output/derived_data/Index.noindex/DataStore", "--verbose", "--disable-update-check", "--quiet", "--format", "checkstyle"] exited with status code 1. error: Shell command '/usr/bin/env xcodebuild -project /Users/test/Some.xcodeproj -list -json' returned exit status '74':

[2022-11-29T04:30:20.751Z] [20:30:20]: ▸ . Updating the Danger gem might fix the issue. Your Danger version: 9.0.0, latest Danger version: 9.1.0

[2022-11-29T04:30:20.751Z] [20:30:20]: ▸ \e[0m

[2022-11-29T04:30:20.751Z] [20:30:20]: ▸ #  from PeripheryDangerfile:5

[2022-11-29T04:30:20.751Z] [20:30:20]: ▸ #  -------------------------------------------

[2022-11-29T04:30:20.751Z] [20:30:20]: ▸ #

[2022-11-29T04:30:20.751Z] [20:30:20]: ▸ >  periphery.scan(

[2022-11-29T04:30:20.751Z] [20:30:20]: ▸ #      config: ".periphery.yml",

[manicmaniac]

The error logs of periphery is written in your log. Isn't it enough?

Shell command '/usr/bin/env xcodebuild -project /Users/test/Some.xcodeproj -list -json' returned exit status '74'

This error is from here https://github.com/peripheryapp/periphery/blob/2173f0d5cb09bf5708776224b0b56f239dbff3f5/Sources/XcodeSupport/Xcodebuild.swift#L76

[shkhaliq]

@manicmaniac Not really as exit code 74 is really broad. Seems like we don't spit out the stdout here(https://github.com/manicmaniac/danger-periphery/blob/4fd02a1f16830d7b300b28906f9ad72ce9a463e2/lib/periphery/runner.rb#L16)

when the exception is raised so all the detailed logs is never printed out.

Also this is possibly a separate issue but sending down verbose: true is not respected as I don't see the verbose periphery logs either

[manicmaniac]

Hmm... let me get this straight.

Shell command '/usr/bin/env xcodebuild -project /Users/test/Some.xcodeproj -list -json' returned exit status '74'

This message includes all output from stderr. I agree exit status '74' is so broad but that is the raw text from Periphery. The former part xcodebuild -project /Users/test/Some.xcodeproj -list -json looks more important in this case.

when the exception is raised so all the detailed logs is never printed out.

As you say danger-periphery suppresses stdout once an error occurs. However, about this case, I don't think analyzing stdout helps your debug because Periphery dies in early stage and I guess it writes nothing meaningful to stdout.

---

Also this is possibly a separate issue but sending down verbose: true is not respected as I don't see the verbose periphery logs either

Yes, passing verbose: true option to periphery.scan is passed through to Periphery executable so danger-periphery doesn't recognize it. In other words, there's no "verbose" option in danger-periphery now.

---

Generally speaking, I agree that danger-periphery might have to print stdout from Periphery for more complex error. But ususally stdout log is much bigger than stderr log so simply adding stdout log to raised error doesn't sound nice.

If you have a good idea, I'm welcome to hear that or you can open a pull request.

[mildm8nnered]

"Yes, passing verbose: true option to periphery.scan is passed through to Periphery executable so danger-periphery doesn't recognize it. In other words, there's no "verbose" option in danger-periphery now."

It would be nice to be able to override this. I'm trying to debug an issue right now where I see no output at all on my CI machines, while running danger locally detects one violation.

It's hard to tell if periphery is even running on the CI machines.

[manicmaniac]

I think passing the --verbose option to Periphery is difficult because it makes it hard for danger-periphery to parse the output. So, danger-periphery internally adds the --quiet option to Periphery.

However, this PR may resolve this issue. It introduces periphery.verbose = true option, which allows printing the entire output from Periphery.

I will close this issue, but feel free to reopen it if you still experience any inconvenience.