oven-sh / bun

Incredibly fast JavaScript runtime, bundler, test runner, and package manager – all in one
https://bun.sh
Other
73k stars 2.66k forks source link

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

Closed RiskyMH closed 3 weeks ago

RiskyMH commented 4 months ago

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:

paperdave commented 4 months ago

Both paths in https://github.com/oven-sh/bun/blob/e3689e7e83507ae9c63dac7d785e41884daead66/src/cli/run_command.zig#L844-L863

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

sossost commented 4 months ago

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

ducktype commented 4 months ago
Bun v1.1.5 ([#10383](https://github.com/oven-sh/bun/pull/10383)) on windows x86_64_baseline [UpdateCommand]

Illegal instruction at address 0x7FF65F9E4AB1

- [`tccpp.c:535`](https://github.com/oven-sh/bun/blob/e3689e7e83507ae9c63dac7d785e41884daead66/src/deps/tinycc/tccpp.c#L535): `get_tok_str`
- [`tccpp.c:535`](https://github.com/oven-sh/bun/blob/e3689e7e83507ae9c63dac7d785e41884daead66/src/deps/tinycc/tccpp.c#L535): `get_tok_str`
- [`Allocator.zig:98`](https://github.com/oven-sh/bun/blob/e3689e7e83507ae9c63dac7d785e41884daead66/src/deps/zig/lib/std/mem/Allocator.zig#L98): `rawFree`
- [`semver.zig:724`](https://github.com/oven-sh/bun/blob/e3689e7e83507ae9c63dac7d785e41884daead66/src/install/semver.zig#L724): `orderWithoutTag`
- [`npm.zig:661`](https://github.com/oven-sh/bun/blob/e3689e7e83507ae9c63dac7d785e41884daead66/src/install/npm.zig#L661): `writeFile`
- [`bun.zig:1774`](https://github.com/oven-sh/bun/blob/e3689e7e83507ae9c63dac7d785e41884daead66/src/bun.zig#L1774): `insert`
- ??? at `0x1257d` in /KERNEL32.DLL
- ??? at `0x5aa48` in /ntdll.dll

Features:

<!-- from bun.report: ASCwkpNtWWPcvCe8cWJO -->
paperdave commented 4 months ago

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
paperdave commented 4 months ago

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

sossost commented 4 months ago

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

RiskyMH commented 3 weeks ago

Looks like this doesn't crash anymore!

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