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

bunx bun in "access denied" folder crashes but should give better error #10472

Closed
RiskyMH opened this issue Apr 24, 2024 · 7 comments
Closed
Labels
bug Something isn't working good first issue Something that would be good for new contributors

Comments

@RiskyMH
Copy link
Member

RiskyMH commented Apr 24, 2024

How can we reproduce the crash?

  1. cd "C:\Documents and Settings"
  2. bunx bun

It makes sense that it errors, but @paperdave said there should be a better message then just crash.
Also in a way as I'm using bunx it may not need to have access to the folder?

Relevant log output

No response

Stack Trace (bun.report)

Bun v1.1.5 (#10383) on windows x86_64 [BunxCommand]

error: CouldntReadCurrentDirectory

Features:

@RiskyMH RiskyMH added bug Something isn't working crash An issue that could cause a crash labels Apr 24, 2024
@paperclover
Copy link
Member

Both paths in

bun/src/cli/run_command.zig

Lines 844 to 863 in e3689e7

const root_dir_info = this_bundler.resolver.readDirInfo(this_bundler.fs.top_level_dir) catch |err| {
if (!log_errors) return error.CouldntReadCurrentDirectory;
if (Output.enable_ansi_colors) {
ctx.log.printForLogLevelWithEnableAnsiColors(Output.errorWriter(), true) catch {};
} else {
ctx.log.printForLogLevelWithEnableAnsiColors(Output.errorWriter(), false) catch {};
}
Output.prettyErrorln("<r><red>error<r><d>:<r> <b>{s}<r> loading directory {}", .{ @errorName(err), bun.fmt.QuotedFormatter{ .text = this_bundler.fs.top_level_dir } });
Output.flush();
return err;
} orelse {
if (Output.enable_ansi_colors) {
ctx.log.printForLogLevelWithEnableAnsiColors(Output.errorWriter(), true) catch {};
} else {
ctx.log.printForLogLevelWithEnableAnsiColors(Output.errorWriter(), false) catch {};
}
Output.prettyErrorln("error loading current directory", .{});
Output.flush();
return error.CouldntReadCurrentDirectory;
};

should print some better text than "error loading current directory"

then in crash_handler.zig's handleRootError, the error can be ignored, similar to how InvalidArgument doesnt print anything. an alternative is to do all the error printing there, however i think the better message will be within the configureEnvForRun function

@paperclover paperclover added good first issue Something that would be good for new contributors and removed crash An issue that could cause a crash labels Apr 24, 2024
@sossost
Copy link

sossost commented Apr 24, 2024

Hello, @paperdave I am sossost, a new contributor. Can I solve this problem??

@ducktype

This comment was marked as off-topic.

@paperclover
Copy link
Member

Hello, @paperdave I am sossost, a new contributor. Can I solve this problem??

sure. i have outlined the code above which is the origin of the error. in riskymh's reproduction, the second case is hit, indicating readDirInfo returned null. it might be possible to extract a better error code from this (is it file not found or access denied? im not really sure how the semantics of the C:\Documents and Settings compatibility hack).

not sure how to reproduce this on macOS and Linux, but i've noticed another issue which is if you are not allowed to read the cwd, you get Unexpected.

image

In debug:
image

@paperclover
Copy link
Member

note, i dont think we can/should fix the second case. that seems to be a bug in the zig standard library.

@sossost
Copy link

sossost commented Apr 25, 2024

@paperdave I modified code based on your advice. Did I write this correctly?

@RiskyMH
Copy link
Member Author

RiskyMH commented Aug 17, 2024

Looks like this doesn't crash anymore!

C:\Documents and Settings>bunx bun
error loading current directory
error: An internal error occurred (CouldntReadCurrentDirectory)

@RiskyMH RiskyMH closed this as completed Aug 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Something that would be good for new contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants