Improve local shell CWD setting

[Nokel81]

Signed-off-by: Sebastian Malton [email protected]

Just some quality of life improvements.

[jansav]

Shouldn't suppress unknown exceptions here. It hides information, prevents centralized error handling and logging etc.

[Nokel81]

How is this suppressing unknown errors, do you mean I should handle each FS error code separately?

[jansav]

I mean that unknown exceptions are unknown for a reason. We should treat them as fatal exceptions which have to be fixed from our side.

So basically, separate fatal exceptions and non fatal exceptions from each other.

• Fatal exception is something unexpected that we need to know about. User has no way to fix it.
• Non fatal exception is something expected that we don't need to know about. User has to fix it, since it's part of the non-happy case scenario. In this case, e.g. providing path to directory that doesn't exist is "non fatal exception", since user can simply change the directory. (as you have done)

I can see that you have done changes to the logic handling exceptions, and it's getting better. Most important part for now, is that the unknown exceptions are reported somewhere so that we can start treating them as bugs to be fixed.

In the future, I think that we should start catching the fatal exceptions from top level and using throw as a mechanism since then we have most detailed error stack available when we are trying to fix the issue. But yeah, that's long in the future, since it would mean that we need to get rid of larger scope "try/catch"-usages.

[Nokel81]

We cannot really treat them as fatal since the application should continue to be usable, I would prefer "expected exceptions" vs "unexpected exceptions" (which I know sounds like an oxymoron).

In this I don't really like the design of JS, and much prefer Rust's design, where errors are just data. It is slightly hard to do that in JS since the entire ecosystem uses throw

[jansav]

There are no unit tests. We (me and @Iku-turso) are eager to help in making this happen :)

[Nokel81]

@jansav I have added some basic unit tests.

[jansav]

@jansav I have added some basic unit tests.

Great!

[jansav]

I don't see the big picture behind the improvements (e.g. why these changes are introduced?), so someone else should check it too.

Anyway, changes that I pointed out are done, so I'm happy.

[Nokel81]

I don't see the big picture behind the improvements (e.g. why these changes are introduced?), so someone else should check it too.

I goal of this code was two fold:

1. Allow the user to use a folder picker for the path instead of having to type it out manually (and the clear button felt like a natural extension of that)
2. Improve the handling of CWD settings for the local shell sessions so that users never get chdir errors.