Return error code when process exits with error

[marcospb19]

Current behavior

With the exception of this metadata error that usually occurs when you're not running inside a project:

https://github.com/holmgr/cargo-sweep/blob/475ac9ec16518596fc21cb2ea77bd8045b5a4a6d/src/main.rs#L154-L157

For every possible error in cargo-sweep, it reports the error and exits with a success exit code.

Expected behavior

The cargo-sweep process should exit with an error exit code.

[marcospb19]

Some technical details for someone who wants to pick this:

I suggest you move contents from main to a new run function, and execute it from main, then use the error! macro in case Result::Err is returned from run.

Fixing on main will break some integration tests that panic when they see an error exit code.

[jyn514]

I suggest you move contents from main to a new run function, and execute it from main, then use the error! macro in case Result::Err is returned from run.

Fixing on main will break some integration tests that panic when they see an error exit code.

I don't agree with this, I think we should continue to use anyhow::Error in main to report errors. To fix the tests we can just update them to look at stderr instead of stdout.

See also https://github.com/holmgr/cargo-sweep/pull/88#discussion_r1096578892.

[marcospb19]

@jyn514 but we can't return anyhow::Error in main, because we are using error! to report our errors and not Debug.

Why call error! for each error instead of having the caller receive it with .context() and have just one place for reporting? It's pretty common to have functions return an error with all the details to be reported.

[jyn514]

@jyn514 but we can't return anyhow::Error in main, because we are using error! to report our errors and not Debug.

Why call error! for each error instead of having the caller receive it with .context() and have just one place for reporting? It's pretty common to have functions return an error with all the details to be reported.

I don't think we should be using error! though - if you look at the current code, we're inconsistent about what uses error!, e.g. https://github.com/holmgr/cargo-sweep/blob/c4c5e19dffe36b86bd6c76b186d40b8fcc1de53f/src/main.rs#L184-L186 and what returns an error from main, e.g. https://github.com/holmgr/cargo-sweep/blob/c4c5e19dffe36b86bd6c76b186d40b8fcc1de53f/src/main.rs#L154-L162

I guess standardizing on error! instead of main is fine, but it does seem like a bit of extra work for little benefit.

[marcospb19]

I guess standardizing on error! instead of main is fine

By main you mean Err returning on main, right?

My suggestion of wrapping stuff around a run function is just if you choose to be consistent using error!(), but of course, it doesn't make sense for what you suggested (returning Result on main, but that changes how most errors are currently reported).

[Expurple]

Shouldn't this issue be closed by now? I can't reproduce exit code 0 on errors. Some of the erroring commands I've tried:

• cargo sweep -r -t 30 -i from #111
• cargo sweep --file from #117
• cargo sweep --stamp name_here from #118

[marcospb19]

I found some https://github.com/holmgr/cargo-sweep/blob/507575643ff344cfd0326a8d13ec0b6836f16fda/src/main.rs#L187-L190