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

Pass down error log #76

Open
shkhaliq opened this issue Nov 29, 2022 · 4 comments
Open

Pass down error log #76

shkhaliq opened this issue Nov 29, 2022 · 4 comments
Assignees

Comments

@shkhaliq
Copy link

shkhaliq commented Nov 29, 2022

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
Copy link
Owner

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
Copy link
Author

shkhaliq commented Dec 1, 2022

@manicmaniac Not really as exit code 74 is really broad. Seems like we don't spit out the stdout here(

raise "error: #{arguments} exited with status code #{status.exitstatus}. #{stderr}" unless status.success?
)

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
Copy link
Owner

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
Copy link

mildm8nnered commented Mar 31, 2024

"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 manicmaniac self-assigned this Apr 11, 2024
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