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

[bug] Update fails on Windows when constructing the updater using the UpdaterBuilder directly #2335

Open
talzion12 opened this issue Jan 19, 2025 · 7 comments · May be fixed by #2434
Open
Labels
bug Something isn't working plugin: updater

Comments

@talzion12
Copy link

I'm using the UpdaterBuilder to construct an Updater and initiate an update but I'm getting a panic error: range start index 1 out of range for slice of length 0 in tauri-plugin-updater-2.3.1\src\updater.rs:616:52.

It seems to be because the UpdaterBuilder::current_exe_args is empty and it's not possible to change it from outside the crate.

The reason I'm using the UpdaterBuilder directly and not using UpdaterExt is because I only want to use the update functionality but I'm otherwise not using tauri and I don't want to construct a Runtime. Is this kind of use not supported?

@FabianLars FabianLars added bug Something isn't working plugin: updater labels Jan 19, 2025
@FabianLars
Copy link
Member

Is this kind of use not supported?

It's indeed not a use-case we considered yet for this plugin. But even for tauri apps this is something we need to fix because UpdaterBuilder is exposed and it's just "recommended" to use updater_builder()

@amrbashir
Copy link
Member

@talzion12 if you don't use tauri, maybe take a look at cargo-packger and its own updater.

@talzion12
Copy link
Author

@amrbashir I will, thanks!

@talzion12
Copy link
Author

I migrated my code to cargo-packager but ultimately decided to revert and stick with Tauri for my project. For now I created a fork that simply makes the UpdateBuilder::current_exe_args function public so I can override the exe args from my project and avoid the panic. However I don't think this is the right solution because the possibility of a panic is a bug in my opinion.

I see 2 possible solutions to not panic when current_exe_args is not set:

  1. Pass an empty LAUNCHAPPARGS argument to the msi/nsis.
  2. Get the args from std::env::args.

Let me know what you think and if you have another solution in mind.

@amrbashir
Copy link
Member

I think we can just fix the underlying issue of indexing an array without checking its length, we should even skip passing LAUNCHAPPARGS and similar flag for NSIS, if there is not arguments to pass.

Moreover I think we can expose relaunch_args method, similar to current_exe_args but better name to indicate its functionality and allow keeping current_exe_args for internal usage .

talzion12 added a commit to talzion12/tauri-plugins-workspace that referenced this issue Feb 17, 2025
@talzion12
Copy link
Author

@amrbashir I created a pr that fixes the panic and also doesn't pass the /ARGS argument for NSIS or the LAUNCHAPPARGS for msi as you suggested.

I wasn't sure how to implement the relaunch_args that you proposed. What happens when there are both current_exe_args and relaunch_args? Do the relaunch_args override it?

This introduces a potentially breaking change in the case that the current_exe_args contained exactly 1 item. Previously the /ARGS would be passed without any arguments and then the installer_args. I'm not familiar with NSIS but I think that this is another bug that this pr fixes because the installer_args would be interpreted as the /ARGS and I think that is not the intended behavior.

Same goes for the LAUNCHAPPARGS that is not passed now. I don't think the MSIs should behave differently based on the existence of this argument, but still I think it could be a breaking change so we may want to keep the older behavior of just passing LAUNCHAPPARGS="".

Let me know what you think.

talzion12 added a commit to talzion12/tauri-plugins-workspace that referenced this issue Feb 17, 2025
@talzion12
Copy link
Author

I see that since updater 2.5.0 UpdaterBuilder::new is private to the crate so this fix won't help me but at least it will fix a potential panic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working plugin: updater
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants