Better error handling

[Ramilito]

Is your feature request related to a problem? Please describe.

We need to improve error handling, today all output is added to the env variable while all errors are swallowed and suppressed.

Describe the solution you'd like

As per @morozov suggestion in #42, we need to make the shell wrapper diffirentiate between stderr and stdout

Describe alternatives you've considered

Displaying the error inside the app and not using the stderr

[Ramilito]

@morozov Just tested this out within the app and it seems to work already for bash automatically, so I removed the error-suppressing handler and included it in #38 Will wait with creating a release to see if it's stable, would love it if you could verify that I haven't messed anything up in fish shell! 🙏

[morozov]

Error handling in Fish works fine. However, kubesess panicking in the case when an item was not selected in the interactive mode doesn't seem appropriate as of ffeb2d020ac16976cbe2478e8141c746b4c41743: https://github.com/Ramilito/kubesess/blob/b3e671b5cb5b778bee28813ba9873e07d1865e05/src/commands.rs#L79-L82

Now it looks like the following from the end-user perspective:

kubesess namespace
# Press ESC
thread 'main' panicked at 'No item selected', src/commands.rs:81:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

It should be fine to quietly exit with a non-zero code here. The shell wrappers will handle the exit code and not update the config. Printing the old config should be no longer necessary either (at least for how the Fish functions are now implemented).

[morozov]

@Ramilito please also see https://github.com/Ramilito/kubesess/pull/45. You may want to include it into the release.

[Ramilito]

@morozov I added a hot-fix for the not selected scenario, tried to avoid printing out the reset but it messed up the environment variable for some reason, using this code at least works:

    if selected_items.is_empty() {
        println!("{}", ORIGINALKUBECONFIG.to_string());
        process::exit(1);
    }

I thought bash wouldn't care about the non-zero exit, but it still did in this case, will modify the shell wrapper for that later and remove the extra print!

[Ramilito]

Merged #45! I'll use this build a day or two and create a release after that if no errors found!

[morozov]

I thought bash wouldn't care about the non-zero exit, but it still did in this case, will modify the shell wrapper for that later and remove the extra print!

The current implementation for Bash indeed doesn't handle the exit codes. It needs to do something like this:

function kc() {
    local OUTPUT
    OUTPUT="$(kubesess ...)" || return $?
    export KUBECONFIG="$OUTPUT"
}

[Ramilito]

Fair enough created an issue for that, will look into it soon and also include a cleanup in that PR!