gleam-lang / gleam

⭐️ A friendly language for building type-safe, scalable systems!
https://gleam.run
Apache License 2.0
16.7k stars 698 forks source link

Add friendly error message when CWD doesn't exist #2876

Closed PgBiel closed 3 months ago

PgBiel commented 3 months ago

When running Gleam commands (for example, gleam clean) from a directory which has since been deleted, the compiler panics, as shown in the message below. It should perhaps display some proper error message instead (e.g. "Error: You're trying to run this command within a directory that does not exist or was removed. Try running this command again in a valid directory.").

error: Fatal compiler bug!

This is a bug in the Gleam compiler, sorry!

Please report this crash to https://github.com/gleam-lang/gleam/issues/new
and include this error message with your report.

Panic: compiler-cli/src/main.rs:566
        Failed to get current directory: FileIo { kind: Directory, action: Open, path: ".", err: Some("No such file or directory (os error 2)") }
Gleam version: 1.0.0
Operating system: linux

If you can also share your code and say what file you were editing or any
steps to reproduce the crash that would be a great help.

You may also want to try again with the `GLEAM_LOG=trace` environment
variable set.
lpil commented 3 months ago

Good idea! Thank you

zahash commented 3 months ago

@lpil can i do this? i have a decent experience in rust.

zahash commented 3 months ago

this is the place where the panic occurs.

// compiler-cli/src/main.rs

fn find_project_paths() -> Result<ProjectPaths> {
    let current_dir = get_current_directory().expect("Failed to get current directory");
    get_project_root(current_dir).map(ProjectPaths::new)
}

is it fine to change the error message from "Failed to get current directory" to "Error: You're trying to run this command within a directory that does not exist or was removed. Try running this command again in a valid directory"

or should i do something more fancy? eg: anyhow::Result has the handy dandy .context("...") method. something like that in gleam_core::error::Error might be useful?

also, i can't reproduce this error on ubuntu. When i cd into a folder, delete it and run gleam, it runs fine. maybe @PgBiel can add some steps on how to reproduce the bug?

also, the function get_current_directory() can also fail if the path is not valid utf8 (which is rare but possible). then the new error message won't be accurate.

i think we should just avoid .expect() and instead propogate the error using ?

zahash commented 3 months ago

gleam won't let me create a project with non utf8 name but i can create a project in a directory where one of the path segments is non-utf8.

eg:

mkdir $'\xC0\xC0'
cd $'\xC0\xC0'

gleam new samplegleam

and when i gleam run the project, it shows this.

error: Fatal compiler bug!

This is a bug in the Gleam compiler, sorry!

Please report this crash to https://github.com/gleam-lang/gleam/issues/new
and include this error message with your report.

Panic: compiler-cli/src/main.rs:558
    Failed to get current directory: NonUtf8Path { path: "/home/zahash/source/\xC0\xC0/samplegleam" }
Gleam version: 1.0.0
Operating system: linux

If you can also share your code and say what file you were editing or any
steps to reproduce the crash that would be a great help.

You may also want to try again with the `GLEAM_LOG=trace` environment
variable set.

well, i wouldn't call it a "compiler bug". and there should instead be better error message saying gleam doesn't support non-utf8 project paths.

but now that i think about it, wouldn't it be nice if it did allow non-utf8 paths? Also, what is the reason behind not allowing non-utf8 project path?

zahash commented 3 months ago

using a ? instead of .expect(...) gives a much better error message

fn find_project_paths() -> Result<ProjectPaths> {
    let current_dir = get_current_directory()?;
    get_project_root(current_dir).map(ProjectPaths::new)
}
error: Non UTF-8 Path Encountered

Encountered a non UTF-8 path, but only UTF-8 paths are supported.

sidenote: maybe this error message could show the actual non-utf8 path.

            Error::NonUtf8Path { path } => {
                let text = format!(
                    "Encountered a non UTF-8 path {}, but only UTF-8 paths are supported.",
                    path.to_string_lossy()
                );
                Diagnostic {
                    title: "Non UTF-8 Path Encountered".into(),
                    text,
                    level: Level::Error,
                    location: None,
                    hint: None,
                }
            }

this will show

error: Non UTF-8 Path Encountered

Encountered a non UTF-8 path /home/zahash/source/��/samplegleam, but only UTF-8 paths are supported.
zahash commented 3 months ago

update: i have found a way to recreate the current working directory not found bug. i just ran this in the project root.

rm -r "$(pwd)"

and then ran the project gleam run. with the ? change, it gives nicer error message.

error: File IO failure

An error occurred while trying to open this directory:

    .

The error message from the file IO library was:

    No such file or directory (os error 2)
zahash commented 3 months ago

anyway, here is a pr https://github.com/gleam-lang/gleam/pull/2886

not sure in which changelog section to describe this change. (maybe Build Tool)

lpil commented 3 months ago

Brilliant, thank you @zahash . The build tool section sounds best to me 👍

zahash commented 3 months ago

@lpil

what about a slight improvement to the error message when non-utf8 paths are encountered? wouldn't showing the actual path be useful?

            Error::NonUtf8Path { path } => {
                let text = format!(
                    "Encountered a non UTF-8 path {}, but only UTF-8 paths are supported.",
                    path.to_string_lossy()
                );
                Diagnostic {
                    title: "Non UTF-8 Path Encountered".into(),
                    text,
                    level: Level::Error,
                    location: None,
                    hint: None,
                }
            }

because the get_current_directory function can fail if the path is non-utf8

pub fn get_current_directory() -> Result<Utf8PathBuf, Error> {
    let curr_dir = std::env::current_dir().map_err(|e| Error::FileIo {
        kind: FileKind::Directory,
        action: FileIoAction::Open,
        path: ".".into(),
        err: Some(e.to_string()),
    })?;
    Utf8PathBuf::from_path_buf(curr_dir.clone()).map_err(|_| Error::NonUtf8Path { path: curr_dir })
}
lpil commented 3 months ago

Nice idea! Thank you

zahash commented 3 months ago

Nice idea! Thank you

@lpil

here is the PR for that https://github.com/gleam-lang/gleam/pull/2895