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

feat(resolver): print detailed error messages for oxc_resolver #4564

Closed
Boshen opened this issue Nov 8, 2023 · 5 comments · Fixed by #4614
Closed

feat(resolver): print detailed error messages for oxc_resolver #4564

Boshen opened this issue Nov 8, 2023 · 5 comments · Fixed by #4614
Labels
A-resolver Area: resolver contrib: easy contrib: has guidance team The issue/pr is created by the member of Rspack.

Comments

@Boshen
Copy link
Contributor

Boshen commented Nov 8, 2023

System Info

None

Details

The current implementation doesn't cover all error messages thrown by oxc_resolver:

fn map_oxc_resolver_error(
error: oxc_resolver::ResolveError,
args: &ResolveArgs<'_>,
plugin_driver: &SharedPluginDriver,
) -> ResolveError {
match error {
oxc_resolver::ResolveError::InvalidPackageTarget(specifier) => {
let message = format!(
"Export should be relative path and start with \"./\", but got {}",
specifier
);
ResolveError(
message.clone(),
Error::Anyhow {
source: anyhow::Error::msg(message),
},
)
}
oxc_resolver::ResolveError::TsconfigNotFound(path) => ResolveError(
format!("{} is not a tsconfig", path.display()),
internal_error!("{} is not a tsconfig", path.display()),
),
_ => {
let is_recursion = matches!(error, oxc_resolver::ResolveError::Recursion);
map_resolver_error(is_recursion, args, plugin_driver)
}
}
}

Notice it only catches 2 errors and falls back to a generic error. oxc_resolver actually throws a lot more errors: https://github.com/web-infra-dev/oxc/blob/7d85492a03e34fec11e27eae42db28a7c3f12e16/crates/oxc_resolver/src/error.rs#L6

It would be nice to cover and print nice error messages for all of them.

Reproduce link

No response

Reproduce Steps

To get started, try Rspack with the experiments.rspackFuture.new_resolver = true configuration and then introduce any of the oxc_resolver errors in your setup.

@github-actions github-actions bot added the team The issue/pr is created by the member of Rspack. label Nov 8, 2023
@msdlisper
Copy link
Contributor

hi~ I need some help :), I try to deal with oxc_resolver: : ResolveError: : IOError errors, found the inside of the rspack Error: : Io is the need to accept theio: : Errortypes, like this:

   oxc_resolver::ResolveError::IOError(error) => ResolveError(
      format!("IOError"),
      Error::Io {
        source: error.0.as_ref().unwrap(), // or error.into()
      },
    ),

But I found that oxc lacked such an implementation:
https://github.com/oxc-project/oxc/blob/7d85492a03e34fec11e27eae42db28a7c3f12e16/crates/oxc_resolver/src/error.rs#L124

like this:

impl Into<io::Error> for ResolveError

Is that true? Or is there another solution?

@Boshen
Copy link
Contributor Author

Boshen commented Nov 10, 2023

https://github.com/oxc-project/oxc/blob/7d85492a03e34fec11e27eae42db28a7c3f12e16/crates/oxc_resolver/src/error.rs#L124

Feel free to add the From / Into trait in oxc_resolver as well, and link it with oxc_resolver = { path = "..." } in rspack_core/Cargo.toml to test it out.

Otherwise give me your work-in-progress branch and I'll add the trait.

@msdlisper
Copy link
Contributor

I'll have a try

@msdlisper
Copy link
Contributor

I have successfully debugging oxc_resolver locally, but after all, this needs to update the oxc-project/oxc code, so I want to leave a TODO here, and first handle ResolveError::IOError, Later I will go to oxc-project /oxc to submit a pr and come back to solve this TODO

@Boshen
Copy link
Contributor Author

Boshen commented Nov 11, 2023

I have successfully debugging oxc_resolver locally, but after all, this needs to update the oxc-project/oxc code, so I want to leave a TODO here, and first handle ResolveError::IOError, Later I will go to oxc-project /oxc to submit a pr and come back to solve this TODO

I can make a quick release after your PR in oxc 😁

Boshen pushed a commit to oxc-project/oxc that referenced this issue Nov 11, 2023
github-merge-queue bot pushed a commit that referenced this issue Nov 12, 2023
#4614)

* feat(resolver): print detailed error messages for oxc_resolver (#4564)

* fix(resolver): print out the IOError of oxc

* chore(resolver): Modify as prompted by clippy

* fix(resolver): process path_no_found type error
Boshen pushed a commit to oxc-project/oxc-resolver that referenced this issue Dec 6, 2023
Boshen pushed a commit to oxc-project/oxc-resolver that referenced this issue Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-resolver Area: resolver contrib: easy contrib: has guidance team The issue/pr is created by the member of Rspack.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants