Issue #109 | Improving errors #184

MariaArtemis · 2022-09-22T15:08:39Z

Alright, so this is the work I've done so far. It's not in a complete stage yet, and some more work needs to be done. I think I could finish whatever changes are needed tomorrow, but a solution needs to be found for one stark problem, and that it can be very hard to know exactly where and what broke when these happen, and I think we can fix that by:

Enforce a "standard" error message. This could be something like "Function run() | src/run.rs | 'Failed to start up' "
Add a String to each enum so that we can print out said string
Use more error matching the like so we can solve errors on the spot than piping them up if we don't absolutely need to pipe them up.

Sidenote: I'm glad this project is using MIT. Maybe we can add a file called CONTRIBUTORS adding everyone's individual copyright license, but I digress, it's not important for me.

Also, the majority of files were edited, but the majority is just converting the new function. The majority of changes happened in main.rs and errors.rs.

There are failing tests but that can be fixed at the end, I'm guessing the majority of them is just not made for the new kinds of errors.

A new error type has been created for the binary part of the tool, and the project should compile and work. It works with printing out the errors, and works just like any good old Rust error. There is however one weak point, and that is debugging is hard with this method. This should be solved before this is merged into main.

cnpryer

Hmm. It's quite nice. Good job! I'm in the middle of some unrelated work. So I'd like to come back and play with it. But so far this is looking great!

A couple quick changes I spotted can be resolved though.

.gitignore

src/bin/huak/main.rs

.gitignore

Co-authored-by: Chris Pryer <[email protected]>

MariaArtemis · 2022-09-22T15:20:08Z

Cheers.

I'll go through all of the requests tomorrow when I'm done with school.

src/bin/huak/main.rs

cnpryer · 2022-09-22T15:30:10Z

Cheers.

I'll go through all of the requests tomorrow when I'm done with school.

Take your time :)

I tried running it but couldn't. Looks like the ci-rust action is failing. Just a heads up.

MariaArtemis · 2022-09-22T15:35:05Z

Thats odd. It runs fine for me. I'll have a look at that

cnpryer · 2022-09-22T15:37:35Z

I just gh pr checkout 184 from master and it worked. Odd.

MariaArtemis · 2022-09-22T15:39:09Z

Oh the CI fails cus some tests fail. Mentioned that in the original post. I'll be fixing those tmr

Co-authored-by: Chris Pryer <[email protected]>

MariaArtemis · 2022-09-23T15:17:47Z

I got errands today but I'll do some work tonight. If you go back to original post, what do you think about improving these errors with more debug information?

cnpryer · 2022-09-23T15:23:09Z

I got errands today but I'll do some work tonight. If you go back to original post, what do you think about improving these errors with more debug information?

No prob. I appreciate the communication.

I'd say let's try to implement this in pieces. We can keep PRs smaller. So a follow up one can add debug info. This one we should just improve on the current error handling. I like what you have here, I just want us to clarify how we'll handle the logic I called out in my most recent comment.

So

Add a String to each enum so that we can print out said string

this I like. I just think we need something for when it's an error code/info from a process we're wrapping (like ruff's).

Use more error matching the like so we can solve errors on the spot than piping them up if we don't absolutely need to pipe them up.

this I also like. but we should approach it in pieces.

cnpryer

This comment specifically

src/huak/errors.rs

MariaArtemis · 2022-09-23T15:25:34Z

Thanks, I'll have a look at it.

MariaArtemis · 2022-09-23T20:34:03Z

Oh the CI fails cus some tests fail. Mentioned that in the original post. I'll be fixing those tmr

I'm looking at the error and its giving me "No such file or directory (os error 2)" errors. I have no clue what this means, but I'll be digging through whats failing.

and the option for Ruff, PyBlack and PyTest errors can be done. Fixed .gitignore too.

src/huak/errors.rs

Co-authored-by: Chris Pryer <[email protected]>

MariaArtemis · 2022-09-23T21:53:21Z

Okay, it compiles and works now, and I (think) have implemented what you wanted changed. The only problem is the tests, which I don't even know my changes to the error type makes it fail, considering its an OS error?

cnpryer · 2022-09-23T21:54:44Z

I see cargo fmt --check is failing.

MariaArtemis · 2022-09-23T21:56:24Z

I think I need to fix my IDE's pushing then, it should do cargo fmt before my commit but apparently not. Tests still fail.

cnpryer · 2022-09-23T21:57:30Z

If you're talking about pre-commit it's only running --check. You need to run cargo fmt. cargo clippy too.

cnpryer · 2022-09-23T21:58:33Z

More info on the checks at the bottom of this section.

MariaArtemis · 2022-09-23T22:02:27Z

Yeah, was just me misunderstanding what "Reformat code" did in IntelliJ, but I'll remember to fmt now. The linter is not happy, but I don't know how to run that locally.

MariaArtemis · 2022-09-23T22:04:35Z

However, after fixing what the linter complains about, the main problem is just tests. I still do not know what makes them fail, but the errors I get is this

---- ops::clean::tests::clean stdout ----
resource archive does not exist
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: CliError { error: AnyHowError(No such file or directory (os error 2)) }', src/huak/utils/test_utils.rs:15:19

This happens on:

failures:
    env::venv::tests::from
    ops::clean::tests::clean
    ops::fmt::tests::fmt
    ops::install::tests::installs_dependenices
    ops::new::tests::creates_project
    ops::remove::tests::removes_dependencies
    project::tests::from

cnpryer · 2022-09-23T22:14:15Z

However, after fixing what the linter complains about, the main problem is just tests. I still do not know what makes them fail, but the errors I get is this...

heh. A great reason to welcome PRs like these 😅

cnpryer · 2022-09-23T22:14:51Z

I can try to help out in a bit if you need it. when we have better errors we'll have less trouble debugging something like this.

MariaArtemis · 2022-09-23T22:17:56Z

Debugging help would be nice. I do not know how it doesn't fail on the current main branch, so SOMETHING has changed between current main and my dev branch. I'll be trying to fix this now.

MariaArtemis · 2022-09-23T22:41:58Z

Could it be the tests themselves are faulty? It's erroring on the fact that there are no files, which doesn't have anything to do with my contribution. The only things I have changed myself is how errors are handled. Very weird error.

cnpryer · 2022-09-23T22:45:34Z

Could the tests be faulty

For sure. This entire project was started this month lol. I should be able to help out soon.

MariaArtemis · 2022-09-23T22:47:34Z

I've digged through the edited files and I see nothing that could change the behavior of functions. ¯\_(ツ)_/¯

cnpryer · 2022-09-23T22:50:36Z

What OS are you on?

MariaArtemis · 2022-09-23T22:51:47Z

I'm running Pop!_OS 22.04.

cnpryer · 2022-09-23T22:56:32Z

with

env::venv::tests::from

~~failing I think it's something to do with the mock strategy for tests~~ I think it's when it's attempting to create a .venv. but I'm unsure. we're you running the tests locally before?

MariaArtemis · 2022-09-23T22:57:26Z

we're you running the tests locally before?

Not before opening the PR

cnpryer · 2022-09-23T23:00:54Z

Do you have the alias for python?

MariaArtemis · 2022-09-23T23:03:11Z

I don't use aliases, so I'm not sure what you mean by this.

cnpryer · 2022-09-23T23:05:54Z

I'm wondering if it fails here. We should add a MssingPython error or something.

huak/src/huak/env/venv.rs

Line 77 in faaf1bf

crate::utils::command::run_command("python", &args, from)?;

See #160

MariaArtemis · 2022-09-23T23:08:47Z

Pushed this so the linter is happy. If the tests fail there, then idk

cnpryer · 2022-09-23T23:08:53Z

If you use discord it might help to chat more there https://discord.gg/St3menxFZT

src/huak/env/venv.rs

Adrian Berg added 2 commits September 22, 2022 16:52

Minor edit

e923192

cnpryer requested changes Sep 22, 2022

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

src/bin/huak/main.rs Outdated Show resolved Hide resolved

cnpryer reviewed Sep 22, 2022

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

Update src/bin/huak/main.rs

49806b3

Co-authored-by: Chris Pryer <[email protected]>

cnpryer reviewed Sep 22, 2022

View reviewed changes

src/bin/huak/main.rs Outdated Show resolved Hide resolved

Update src/bin/huak/main.rs

4485a08

Co-authored-by: Chris Pryer <[email protected]>

cnpryer reviewed Sep 23, 2022

View reviewed changes

src/huak/errors.rs Outdated Show resolved Hide resolved

cnpryer force-pushed the master branch from 3b682c6 to 3de62ff Compare September 23, 2022 16:19

Adrian Berg added 2 commits September 23, 2022 23:41

Changed from CliErrorCode to CliErrorType, prints out anyhow errors now,

76ce6b7

and the option for Ruff, PyBlack and PyTest errors can be done. Fixed .gitignore too.

Merge branch 'master' into master

507219a

cnpryer reviewed Sep 23, 2022

View reviewed changes

src/huak/errors.rs Show resolved Hide resolved

cnpryer reviewed Sep 23, 2022

View reviewed changes

src/huak/errors.rs Outdated Show resolved Hide resolved

Adrian Berg and others added 2 commits September 23, 2022 23:49

Update src/huak/errors.rs

42a6961

Co-authored-by: Chris Pryer <[email protected]>

Fixed main.rs after merging with the updated main branch.

1b66985

Formatting

7108517

cnpryer mentioned this pull request Sep 23, 2022

Status codes and improving error handling #207

Closed

Making the linter happy

4e56937

cnpryer reviewed Sep 23, 2022

View reviewed changes

src/huak/env/venv.rs Outdated Show resolved Hide resolved

cnpryer approved these changes Sep 23, 2022

View reviewed changes

cnpryer merged commit 8a03059 into cnpryer:master Sep 23, 2022

cnpryer mentioned this pull request Sep 23, 2022

Handle python alias #160

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #109 | Improving errors #184

Issue #109 | Improving errors #184

MariaArtemis commented Sep 22, 2022 •

edited by cnpryer

Loading

cnpryer left a comment •

edited

Loading

MariaArtemis commented Sep 22, 2022

cnpryer commented Sep 22, 2022

MariaArtemis commented Sep 22, 2022

cnpryer commented Sep 22, 2022 •

edited

Loading

MariaArtemis commented Sep 22, 2022

MariaArtemis commented Sep 23, 2022

cnpryer commented Sep 23, 2022

cnpryer left a comment

MariaArtemis commented Sep 23, 2022

MariaArtemis commented Sep 23, 2022

MariaArtemis commented Sep 23, 2022

cnpryer commented Sep 23, 2022

MariaArtemis commented Sep 23, 2022

cnpryer commented Sep 23, 2022

cnpryer commented Sep 23, 2022

MariaArtemis commented Sep 23, 2022

MariaArtemis commented Sep 23, 2022

cnpryer commented Sep 23, 2022

cnpryer commented Sep 23, 2022 •

edited

Loading

MariaArtemis commented Sep 23, 2022

MariaArtemis commented Sep 23, 2022

cnpryer commented Sep 23, 2022

MariaArtemis commented Sep 23, 2022 •

edited

Loading

cnpryer commented Sep 23, 2022

MariaArtemis commented Sep 23, 2022

cnpryer commented Sep 23, 2022 •

edited

Loading

MariaArtemis commented Sep 23, 2022

cnpryer commented Sep 23, 2022

MariaArtemis commented Sep 23, 2022

cnpryer commented Sep 23, 2022 •

edited

Loading

MariaArtemis commented Sep 23, 2022

cnpryer commented Sep 23, 2022

Issue #109 | Improving errors #184

Issue #109 | Improving errors #184

Conversation

MariaArtemis commented Sep 22, 2022 • edited by cnpryer Loading

cnpryer left a comment • edited Loading

Choose a reason for hiding this comment

MariaArtemis commented Sep 22, 2022

cnpryer commented Sep 22, 2022

MariaArtemis commented Sep 22, 2022

cnpryer commented Sep 22, 2022 • edited Loading

MariaArtemis commented Sep 22, 2022

MariaArtemis commented Sep 23, 2022

cnpryer commented Sep 23, 2022

cnpryer left a comment

Choose a reason for hiding this comment

MariaArtemis commented Sep 23, 2022

MariaArtemis commented Sep 23, 2022

MariaArtemis commented Sep 23, 2022

cnpryer commented Sep 23, 2022

MariaArtemis commented Sep 23, 2022

cnpryer commented Sep 23, 2022

cnpryer commented Sep 23, 2022

MariaArtemis commented Sep 23, 2022

MariaArtemis commented Sep 23, 2022

cnpryer commented Sep 23, 2022

cnpryer commented Sep 23, 2022 • edited Loading

MariaArtemis commented Sep 23, 2022

MariaArtemis commented Sep 23, 2022

cnpryer commented Sep 23, 2022

MariaArtemis commented Sep 23, 2022 • edited Loading

cnpryer commented Sep 23, 2022

MariaArtemis commented Sep 23, 2022

cnpryer commented Sep 23, 2022 • edited Loading

MariaArtemis commented Sep 23, 2022

cnpryer commented Sep 23, 2022

MariaArtemis commented Sep 23, 2022

cnpryer commented Sep 23, 2022 • edited Loading

MariaArtemis commented Sep 23, 2022

cnpryer commented Sep 23, 2022

MariaArtemis commented Sep 22, 2022 •

edited by cnpryer

Loading

cnpryer left a comment •

edited

Loading

cnpryer commented Sep 22, 2022 •

edited

Loading

cnpryer commented Sep 23, 2022 •

edited

Loading

MariaArtemis commented Sep 23, 2022 •

edited

Loading

cnpryer commented Sep 23, 2022 •

edited

Loading

cnpryer commented Sep 23, 2022 •

edited

Loading