Implement better error system and segregate errors and commands #23
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: quexeky <git@quexeky.dev>
Signed-off-by: quexeky <git@quexeky.dev>
Signed-off-by: quexeky <git@quexeky.dev>
…ective commands.rs files Signed-off-by: quexeky <git@quexeky.dev>
Signed-off-by: quexeky <git@quexeky.dev>
Signed-off-by: quexeky <git@quexeky.dev>
Signed-off-by: quexeky <git@quexeky.dev>
Signed-off-by: quexeky <git@quexeky.dev>
…commands return UserValue struct Signed-off-by: quexeky <git@quexeky.dev>
DecDuck
requested changes
Jan 13, 2025
Member
DecDuck
left a comment
There was a problem hiding this comment.
Ultimately, a good change, but a few concerns:
- a LOT of wrapper types for errors. Can we have some way to imply those or remove them
- I hope you've end to end tested all this, with such a huge refactor. You need to go through and test every single component
src-tauri/src/process/commands.rs
Outdated
| download_type: DownloadType::Game, | ||
| }; | ||
|
|
||
| match process_manager_lock.launch_process(meta) { |
Member
There was a problem hiding this comment.
Why are we launching by DownloadMetadata? This process manager is only for games, and is intentionally designed to only handle one version at a time.
We should be launching by only game_id, to ensure we don't pass garbage parameters to this.
src-tauri/src/process/commands.rs
Outdated
Comment on lines
19
to
32
| let version = match DB | ||
| .borrow_data() | ||
| .unwrap() | ||
| .applications | ||
| .game_statuses | ||
| .get(&id) | ||
| .cloned() | ||
| { | ||
| Some(GameDownloadStatus::Installed { version_name, .. }) => version_name, | ||
| Some(GameDownloadStatus::SetupRequired { .. }) => { | ||
| return Err(ProcessError::SetupRequired).into() | ||
| } | ||
| _ => return Err(ProcessError::NotInstalled).into(), | ||
| }; |
Member
There was a problem hiding this comment.
As per the below comment, this match should really be contained within the process manager, not here.
|
|
||
| let launch_process = game_launcher | ||
| .launch_process( | ||
| &meta, |
Contributor
Author
There was a problem hiding this comment.
Can't get that to work without changing the trait, which is intended to be universal, and thus uses DownloadableMetadata
Contributor
Author
There was a problem hiding this comment.
Scratch that, I've just taken a look again. It's possible to convert it, I just hadn't realised that it was intended as specifically for games. My bad
Contributor
Author
|
Contributor
Author
|
Member
Well the ones I highlighted would have to be used whenever we return an error, rather than propagating one |
Member
I mean with splitting out all the commands and stuff, to make sure all the functions are named properly and work. |
Contributor
Author
I mean, yeah. That's what we did before anyway too. I can make it return only the ErrorKind if you'd like |
Contributor
Author
That's the job of a test suite. Not this PR. But consider it done |
Member
The lack of the aforementioned test suite kinda makes this your problem for now |
Contributor
Author
More pointing out that it would be nice to actually have one later on |
quexeky
commented
Jan 13, 2025
src-tauri/src/lib.rs
Outdated
| debug!("Database is set up"); | ||
|
|
||
| let (app_status, user) = auth::setup().unwrap(); | ||
| let (app_status, user) = auth::setup(); |
Contributor
Author
There was a problem hiding this comment.
I resolved it and I'll keep it resolved because it's only tangential to this, but this function requires the other setup() function to never error out. What do you want me doing with that now that it can error out?
Member
There was a problem hiding this comment.
If it errors out, we should return some sort of SignInNeedsReauth or smth. The function originally errored out, so just copy it from the history.
DecDuck
approved these changes
Jan 13, 2025
quexeky
added a commit
that referenced
this pull request
Jan 25, 2025
* chore(process manager): refactor for generic way to implement cross platform launchers * feat(game): game uninstalling & partial compat * chore(metadata): update metadata * feat(errors): better download manager errors + modal * feat(process): better process management, including running state * feat(downloads): lockless tracking of downloaded chunks * fix(sign on): add message about nonce expiration * feat(download ui): add speed and time remaining information closes #7 Co-authored-by: AdenMGB <140392385+AdenMGB@users.noreply.github.com> * chore: Ran cargo clippy Signed-off-by: quexeky <git@quexeky.dev> * fix(auth initiate): add better error message * feat(auth): offer manual signin * feat(install modal): add note about more install dirs * fix(install flow): clear stale data before requesting new * Delete pages/library.vue * Add files via upload * adds nvm rc! * feat(install modal): add note about more install dirs * fix(install flow): clear stale data before requesting new * Delete pages/library.vue * Add files via upload * fix(library page): fix install button * fix(process): fix poorly designed parsing for executables with spaces * fix(scrollbars): fix ugly scrollbars on edge webview * feat(Compat): Implemented spawning with umu (using umu-wrapper-lib) Signed-off-by: quexeky <git@quexeky.dev> * feat(process manager): Game kill tauri command Signed-off-by: quexeky <git@quexeky.dev> * fix(deep links): Re-enabled deep links Signed-off-by: quexeky <git@quexeky.dev> * feat(process): shared child with stop command * squash(autostart): added adenmgb's autostart feature Squashed commit of the following: commit 085cd94 Author: Aden Lindsay <140392385+AdenMGB@users.noreply.github.com> Date: Mon Dec 30 16:29:41 2024 +1030 Update lib.rs for the DB sync of autostart commit 86f2fb1 Author: Aden Lindsay <140392385+AdenMGB@users.noreply.github.com> Date: Mon Dec 30 16:29:13 2024 +1030 Update db.rs to accomidate the settings sync commit ece11e7 Author: Aden Lindsay <140392385+AdenMGB@users.noreply.github.com> Date: Mon Dec 30 16:27:48 2024 +1030 Update autostart.rs to include DB commit 7ea8a24 Author: Aden Lindsay <140392385+AdenMGB@users.noreply.github.com> Date: Mon Dec 30 15:17:38 2024 +1030 Add files via upload commit af2f232 Author: Aden Lindsay <140392385+AdenMGB@users.noreply.github.com> Date: Mon Dec 30 15:17:09 2024 +1030 Delete src-tauri/Cargo.toml commit 5d27b65 Author: Aden Lindsay <140392385+AdenMGB@users.noreply.github.com> Date: Mon Dec 30 15:15:42 2024 +1030 Add files via upload commit 2eea7b9 Author: Aden Lindsay <140392385+AdenMGB@users.noreply.github.com> Date: Mon Dec 30 15:15:31 2024 +1030 Delete src-tauri/src/lib.rs commit 9a635a1 Author: Aden Lindsay <140392385+AdenMGB@users.noreply.github.com> Date: Mon Dec 30 15:14:49 2024 +1030 Add files via upload commit 2fb0495 Author: Aden Lindsay <140392385+AdenMGB@users.noreply.github.com> Date: Mon Dec 30 15:13:37 2024 +1030 Add files via upload commit ea1be4d Author: Aden Lindsay <140392385+AdenMGB@users.noreply.github.com> Date: Mon Dec 30 15:13:20 2024 +1030 Delete pages/settings/index.vue * fix(download manager): fix incorrect error assumptions & update types * feat(account settings): Add signout functionality (#16) * Create account.vue with logout button * Update auth.rs to add signout command * Update lib.rs to pass sign_out command to frontend * feat(settings): add debug page * Create debug.rs * Update settings.vue to add tab for debug * Update main.scss to add light theme * Update interface.vue to add light mode * Create debug.vue * Update debug.vue too add open log button * Update lib.rs * Update debug.rs * Update debug.rs * Update lib.rs * Update lib.rs * Update debug.rs * Update debug.vue * fix(debug): refactor and cleanup * revert(theme): revert light theming --------- Co-authored-by: DecDuck <declanahofmeyr@gmail.com> * feat(library ui): add installed ui in the library menu * chore(tool manager): Progress on adding tools Going to try changing around the download manager to take a generic trait rather than specifically for game downloads Signed-off-by: quexeky <git@quexeky.dev> * refactor(download manager): Moved download manager to separate directory Signed-off-by: quexeky <git@quexeky.dev> * refactor(download manager): Added Downloadable trait and replaced references to GameDownloadAgent Signed-off-by: quexeky <git@quexeky.dev> * chore(download manager): Renamed most instances of "game" outside of actual game downloads Signed-off-by: quexeky <git@quexeky.dev> * refactor(download manager): Renamed GameDonwloadError to ApplicationDownloadError and moved Signed-off-by: quexeky <git@quexeky.dev> * chore(download manager): Some easy cleanup of the download manager Signed-off-by: quexeky <git@quexeky.dev> * chore(download manager): Ensure that Downloadable is also send and sync Signed-off-by: quexeky <git@quexeky.dev> * refactor(download manager): Moved manifest and stored_manifest to download_manager Signed-off-by: quexeky <git@quexeky.dev> * Revert "refactor(download manager): Moved manifest and stored_manifest to download_manager" This reverts commit 8db2393. * chore(tool manager): Added ToolDownloadAgent Signed-off-by: quexeky <git@quexeky.dev> * chore(download manager): Added manage_queue_signal Signed-off-by: quexeky <git@quexeky.dev> * chore(download manager): Added manage_go_signal command Signed-off-by: quexeky <git@quexeky.dev> * refactor(download manager): Removed all references to anything outside of the DownloadManager Signed-off-by: quexeky <git@quexeky.dev> * refactor(download manager): Fully separate & generic download manager Signed-off-by: quexeky <git@quexeky.dev> * refactor(download manager): Removed Arc requirement for DownloadableMetadata Signed-off-by: quexeky <git@quexeky.dev> * feat(download manager): Added generic download manager Signed-off-by: quexeky <git@quexeky.dev> * fix(game launcher): Renamed game_id to id Signed-off-by: quexeky <git@quexeky.dev> * fix(uninstalling): Re-enabled uninstalling apps Signed-off-by: quexeky <git@quexeky.dev> * refactor(downloads): Moved all files relevant to game downloads to their own directory Signed-off-by: quexeky <git@quexeky.dev> * fix(kill game): Re-enabled killing games Signed-off-by: quexeky <git@quexeky.dev> * feat(recovery): Added database recovery Signed-off-by: quexeky <git@quexeky.dev> * feat(database): Added database corruption dialog Signed-off-by: quexeky <git@quexeky.dev> * chore(README): Updated README.md Signed-off-by: quexeky <git@quexeky.dev> * perf(game downloads): Moved some variable declarations outside of the spawned download thread Signed-off-by: quexeky <git@quexeky.dev> * fix(game downloads): Accidentally was attempting to lock onto something that was already in scope Signed-off-by: quexeky <git@quexeky.dev> * fix(db): Added Settings component Signed-off-by: quexeky <git@quexeky.dev> * refactor: Ran cargo clippy & fmt Signed-off-by: quexeky <git@quexeky.dev> * chore: More cleanup after cargo clippy Also added some type efficiency improvements (using references where possible and added SliceDeque crate) Signed-off-by: quexeky <git@quexeky.dev> * feat(settings): Added max_download_threads setting and separated settings from db Signed-off-by: quexeky <git@quexeky.dev> * chore: Moved generateGameMeta.ts to composables, using PathBuf instead of String for install_dirs Signed-off-by: quexeky <git@quexeky.dev> * chore: General cleanup - Changed some info!() statements to debug!() and warn!() - Removed most Turbofish syntax cases - Removed InvalidCodeError and replaced it with InvalidResponse Signed-off-by: quexeky <git@quexeky.dev> * chore: Removed tests/ Signed-off-by: quexeky <git@quexeky.dev> * chore: Removed tools/ Signed-off-by: quexeky <git@quexeky.dev> * chore: More refining info!() statements Signed-off-by: quexeky <git@quexeky.dev> * feat(download manager): Added UI to change download threads Co-authored-by: AdenMGB <140392385+AdenMGB@users.noreply.github.com> Signed-off-by: quexeky <git@quexeky.dev> * fix(metadata): update routes for new server * fix(handle invalid database): use set_file_name instead of pushing to strings * refactor(compat): remove unnecessary compat code (#20) * Delete pages/settings/compatibility.vue * Update settings.vue * Update debug.vue * Update lib.rs * Update compat.rs * feat(debug): use shift or DEBUG RUST_LOG to show Debug Info * Update settings.vue to have a conditional debug page * Update debug.rs to add RUST_LOG status fetching * Implement better error system and segregate errors and commands (#23) * chore: Progress on amend_settings command Signed-off-by: quexeky <git@quexeky.dev> * chore(errors): Progress on better error handling with segragation of files * chore: Progress on amend_settings command Signed-off-by: quexeky <git@quexeky.dev> * chore(commands): Separated commands under each subdirectory into respective commands.rs files Signed-off-by: quexeky <git@quexeky.dev> * chore(errors): Almost all errors and commands have been segregated * chore(errors): Added drop server error Signed-off-by: quexeky <git@quexeky.dev> * feat(core): Update to using nightly compiler Signed-off-by: quexeky <git@quexeky.dev> * chore(errors): More progress on error handling Signed-off-by: quexeky <git@quexeky.dev> * chore(errors): Implementing Try and FromResidual for UserValue Signed-off-by: quexeky <git@quexeky.dev> * refactor(errors): Segregated errors and commands from code, and made commands return UserValue struct Signed-off-by: quexeky <git@quexeky.dev> * fix(errors): Added missing files * chore(errors): Convert match statement to map_err * feat(settings): Implemented settings editing from UI * feat(errors): Clarified return values from retry_connect command * chore(errors): Moved autostart commands to autostart.rs * chore(process manager): Converted launch_process function for games to use game_id --------- Signed-off-by: quexeky <git@quexeky.dev> * fix(settings): Broken command invoke logic in settings/downloads.vue * feat(logging): Added line numbers to file logging and highlighting to console * chore(progress): Added rolling_progress_updates.rs Signed-off-by: quexeky <git@quexeky.dev> * chore(exit): Progress on cleanup and exit * chore(downloads): Progress on terminator * chore: Progress on rolling progress window * feat(progress): Added rolling progress window Still needs tweaks on specific timings, as well as cleanup * refactor(remote): Created separate function to generate requests * fix(install ui): stop loading on error * fix: fix other metadata endpoints * feat(errors): Using SerializeDisplay for better error management with Result * chore: Update .gitlab-ci.yml * refactor(logging): Using more appropriate logging statements Still probably needs some work, but that's enough for now * chore(logging): Imported appropriate logging macros * Revert "chore: Update .gitlab-ci.yml" This reverts commit fc6bab9. * feat(settings): Allow settings to update UI using fetch_settings command * style(logging): Ensured that all logs start with lowercase capital and have no trailing punctuation * fix(download manager): don't crash download manager if multiple errors come in * feat(downloads): re-enable checksums * fix(logs): add file & line to console logs * fix(ui): modal stack doesn't cover whole app * feat(database): Ensure that any database issues are resolved by standalone functions Functions are as follows: - save_db() - borrow_db_checked() - borrow_db_mut_checked() * chore: Ran cargo clippy & cargo fmt * fix: assorted fixes * fix(download agent): fixed completed indexes * fix: Adding usize to completed_contexts_lock instead of &usize * fix(game downloads): Added error handling for chunk request errors * chore: Apply stashed changes * feat(games): Added multi-argument game launch and setup support * fix: Games not launching due to string semantics * build: Version bump & appimage build * chore: Update .gitlab-ci.yml * Update .gitlab-ci.yml * Update .gitlab-ci.yml with artifacts * feat(settings): Made save button include user feedback & only allow numeric characters * fix(library): Added "LIbrary Failed to Update" content to recover from library load fail * fix(logging): Restored RUST_LOG env functionality * Update changelog.md --------- Signed-off-by: quexeky <git@quexeky.dev> Signed-off-by: DecDuck <declanahofmeyr@gmail.com> Co-authored-by: DecDuck <declanahofmeyr@gmail.com> Co-authored-by: AdenMGB <140392385+AdenMGB@users.noreply.github.com> Co-authored-by: seethruhead <shane.keulen@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.