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

The version check conflicts with shell-config #332

Closed
RyanGreenup opened this issue Feb 25, 2024 · 4 comments
Closed

The version check conflicts with shell-config #332

RyanGreenup opened this issue Feb 25, 2024 · 4 comments

Comments

@RyanGreenup
Copy link

RyanGreenup commented Feb 25, 2024

Warnings in shell-config

On my system:

hoard shell-config -s fish 2>/dev/null

Returns the following:

The supplied trove file is invalid!
missing field `namespaces` at line 2 column 8
A newer Version (v1.4.2) is available at https://github.com/Hyde46/hoard 
Please update.
# Hoard bindings
function __hoard_list
    set hoard_command (hoard --autocomplete list 3>&1 1>&2 2>&3)
    commandline -j $hoard_command
end

if ! set -q HOARD_NOBIND
    bind \ch __hoard_list
end

These warnings should atleast print to STDERR so it can be added to the ~/.config/fish/config.fish like so:

if status is-interactive
   if command -v hoard 1> /dev/null 2>&1
        hoard shell-config -s fish 2>/dev/null | source
    end
end

Crash During Version Check

I kept hitting this .unwrap, I'm not sure why though:

I appreciate that it should parse the json and I'm not able to reproduce this, but it's happened a couple times. Here is the stack trace:

RUST_BACKTRACE=full hoard 2>&1 | xclip -sel clip
Click Me
thread 'main' panicked at /home/ryan/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hoard-rs-1.4.2/src/config.rs:260:14:
called `Option::unwrap()` on a `None` value
stack backtrace:
   0:     0x5596755f2986 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h0d7c32bf461d7ffe
   1:     0x55967561bd70 - core::fmt::write::ha63b5e044fedf034
   2:     0x5596755eec7f - std::io::Write::write_fmt::h0630f8018bef0217
   3:     0x5596755f2764 - std::sys_common::backtrace::print::h123dbd17bf0eb7a1
   4:     0x5596755f3e37 - std::panicking::default_hook::{{closure}}::hadb8daa1dcda954f
   5:     0x5596755f3b99 - std::panicking::default_hook::h8e040ff60ad33519
   6:     0x5596755f42c8 - std::panicking::rust_panic_with_hook::h0da099ccc9df6ce3
   7:     0x5596755f4169 - std::panicking::begin_panic_handler::{{closure}}::h8304f0b6f9a6cbbc
   8:     0x5596755f2e86 - std::sys_common::backtrace::__rust_end_short_backtrace::h3c72b81e025af2eb
   9:     0x5596755f3ef4 - rust_begin_unwind
  10:     0x55967524fe65 - core::panicking::panic_fmt::ha3d303d496008cd4
  11:     0x55967524ff23 - core::panicking::panic::ha3344f4627bec123
  12:     0x55967524fdb6 - core::option::unwrap_failed::h8a484d2e9090c178
  13:     0x55967526a76e - hoard::config::compare_with_latest_version::{{closure}}::h851f8500dbaa2116
  14:     0x55967526c68c - hoard::main::{{closure}}::h11698a08300afde4
  15:     0x55967526b939 - hoard::main::h7954a1e73fc07c49
  16:     0x559675263fa3 - std::sys_common::backtrace::__rust_begin_short_backtrace::h63824d120176940d
  17:     0x55967527f649 - std::rt::lang_start::{{closure}}::ha92790597e96ceef
  18:     0x5596755e7531 - std::rt::lang_start_internal::hd1132dfff4140512
  19:     0x55967526d005 - main
  20:     0x7f718456dc8a - <unknown>
  21:     0x7f718456dd45 - __libc_start_main
  22:     0x5596752506b1 - _start
  23:                0x0 - <unknown>

We could refactor the code to avoid the unwrap, because a version check really shouldn't crash the program:

pub async fn compare_with_latest_version() -> (bool, String) {
    let client = reqwest::Client::builder()
        .user_agent(env!("CARGO_PKG_NAME"))
        .build()
        .unwrap();
    if let Ok(client_response) = client
        .get("https://api.github.com/repos/Hyde46/hoard/releases/latest")
        .send()
        .await
    {
        if let Ok(tag) = client_response.json::<ClientResponse>().await {
            let tag_name = tag.tag_name;
            return (VERSION == &tag_name[1..], tag_name);
        } else {
            return (true, String::new());
        }
    }
    (true, String::new())
}
@Hyde46
Copy link
Owner

Hyde46 commented Feb 25, 2024

Great comments, thanks a lot for the stacktrace.
I like the proposal to not crash if the version check throws an error.

@Hyde46
Copy link
Owner

Hyde46 commented Feb 28, 2024

Should be fixed with #334

@Hyde46 Hyde46 closed this as completed Feb 28, 2024
@RyanGreenup
Copy link
Author

Quick work, thank you for all your hard work!

For the record, I may have figured out why the json wasn't parsing. I might have hit the max open file limit with some podman containers because Alacritty started giving me:

Too many open files (os error 24)

Or maybe not, who knows 🤷

@Hyde46
Copy link
Owner

Hyde46 commented Mar 1, 2024

The parsing error was due to my incompetence in not making sure that trove.yml files still open with the latest state of main. I added some fields, but forgot to add a fallback if they are not present. Now they should be present 🙃

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

2 participants