You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Originally brought up by @thomcc in issue #1222 and somewhat addressed by #1238, it was discussed that maybe cargo-pgrx has some bad UX when it comes to reporting error messages back to the user when things go wrong.
Some points that were brought up:
Maybe we really don't need eyre everywhere within cargo-pgrx. Even though eyre may be easy to use, sometimes it can be overzealous in its output. More output means more confusion to the user, and things may go overlooked
While tracing and derivatives are included in the cargo-pgrx crate, it was noted that any error always outputs a trace. See previous point about why this could be bad. While traces are good to have, we felt that having users "opt-in" to see traces is better than "default is on" for traces. There was discussion about whether or not tracing should even stay in the crate.
Ripping out eyre may mean implementing various Error types throughout the crate, which could be cumbersome (probably why eyre was introduced initially!)
Any kind of default error output should be succinct, to the point, and potentially (if possible!) describe what could be done to address the issue. Of course there are an awful lot of edge cases there, and sometimes the only way to know what needs to be done is to inspect the output of sub-commands that cargo-pgrx utilizes.
Many of the error reporting issues stem from the fact that cargo-pgrx uses std::process::Command frequently to achieve its goals, but the handling of spawning child processes, capturing their stdout/stderr, and possibly piping in stdin can be a bit tricky -- especially if you want to cover as many cases as possible. However, any kind of error reporting overhaul should not be limited to just anything that uses std::process::Command, but should be a crate-wide discovery and overhaul
Any other topics? The purpose of this issue is to collect some thoughts about what an overhaul might look like, and to potentially track any progress made towards that end.
The text was updated successfully, but these errors were encountered:
Originally brought up by @thomcc in issue #1222 and somewhat addressed by #1238, it was discussed that maybe
cargo-pgrx
has some bad UX when it comes to reporting error messages back to the user when things go wrong.Some points that were brought up:
eyre
everywhere withincargo-pgrx
. Even though eyre may be easy to use, sometimes it can be overzealous in its output. More output means more confusion to the user, and things may go overlookedtracing
and derivatives are included in thecargo-pgrx
crate, it was noted that any error always outputs a trace. See previous point about why this could be bad. While traces are good to have, we felt that having users "opt-in" to see traces is better than "default is on" for traces. There was discussion about whether or nottracing
should even stay in the crate.eyre
may mean implementing variousError
types throughout the crate, which could be cumbersome (probably whyeyre
was introduced initially!)cargo-pgrx
utilizes.cargo-pgrx
usesstd::process::Command
frequently to achieve its goals, but the handling of spawning child processes, capturing their stdout/stderr, and possibly piping in stdin can be a bit tricky -- especially if you want to cover as many cases as possible. However, any kind of error reporting overhaul should not be limited to just anything that usesstd::process::Command
, but should be a crate-wide discovery and overhaulAny other topics? The purpose of this issue is to collect some thoughts about what an overhaul might look like, and to potentially track any progress made towards that end.
The text was updated successfully, but these errors were encountered: