biomejs / biome

A toolchain for web projects, aimed to provide functionalities to maintain them. Biome offers formatter and linter, usable via CLI and LSP.
https://biomejs.dev
Apache License 2.0
13.92k stars 421 forks source link

🐛 Internal error when linting using ``pre-commit`` with a ``ThreadPoolBuildError`` #3491

Open DanielNoord opened 1 month ago

DanielNoord commented 1 month ago

Environment information

CLI:
  Version:                      1.8.1
  Color support:                true

Platform:
  CPU Architecture:             x86_64
  OS:                           linux

Environment:
  BIOME_LOG_DIR:                unset
  NO_COLOR:                     unset
  TERM:                         "xterm-256color"
  JS_RUNTIME_VERSION:           unset
  JS_RUNTIME_NAME:              unset
  NODE_PACKAGE_MANAGER:         unset

Biome Configuration:
  Status:                       Loaded successfully
  Formatter disabled:           false
  Linter disabled:              false
  Organize imports disabled:    false
  VCS disabled:                 false

Workspace:
  Open Documents:               0

Note that this is the environment from my personal machine. The error itself occurs sporadically on our CI runners on which I can't run biome rage easily.

What happened?

Run biome format --files-ignore-unknown=true --no-errors-on-unmatched, got:

Biome encountered an unexpected error 00:05
This is a bug in Biome, not an error in your code, and we would appreciate it if you could report it to https://github.com/biomejs/biome/issues/ along with the following information to help us fixing the issue: 00:05
Source Location: crates/biome_cli/src/execute/traverse.rs:150:14 00:05
Thread Name: main 00:05
Message: failed to initialize the global thread pool: ThreadPoolBuildError { kind: IOError(Os { code: 11, kind: WouldBlock, message: "Resource temporarily unavailable" }) }

We run that command via pre-commit with the following hook definition:

      - id: biome
        name: Biome formatting
        entry: biome format --files-ignore-unknown=true --no-errors-on-unmatched
        language: system

Expected result

No error.

Code of Conduct

ematipico commented 1 month ago

This is the first time I see an issue like this, especially an error around the thread pool. I wonder now if your CI allows you to spawn threads.

DanielNoord commented 1 month ago

Other output in the same run is:

Formatted 0 files in 792µs. No fixes needed. 00:05
Formatted 0 files in 3ms. No fixes needed. 00:05
Formatted 1 file in 8ms. No fixes needed. 00:05
Formatted 0 files in 2ms. No fixes needed. 00:05
Formatted 0 files in 770µs. No fixes needed. 00:05
Formatted 1 file in 2ms. No fixes needed. 00:05
Formatted 1 file in 3ms. No fixes needed.

Also, this doesn't happen all the time. I would guess about 10% of the time. I can ask the team maintaining the CI whether we allow spawning threads, but wouldn't this then be an issue for all CI runs?

ematipico commented 1 month ago

Maybe this issue can shed some light on the matter. However, I think this is an issue around the CI environment: https://github.com/rust-lang/rust/issues/69140

jaapdeheer commented 1 month ago

Heya, colleague of @DanielNoord (and complete Rust noob) here.

For context, our CI runs on a beefy machine with 192 logical CPUs.

If I understand the biome code and the thread pool docs correctly, the thread pool in biome is constructed without specifying num_threads, meaning:

the ... number of threads ... is based on the RAYON_NUM_THREADS environment variable (if set), or the number of logical CPUs (otherwise).

We don't have RAYON_NUM_THREADS set, so I suspect it's using 192 threads. Often we're running multiple CI runs at the same time, so if it randomly happens that we have a bunch of biome instances running concurrently, maybe we're just hitting some limit to the number of threads, or some other resource limit.

If this is what's happening, then we could work around it by setting RAYON_NUM_THREADS much lower. Maybe it makes sense for biome to default to a more conservative maximum number of threads, and/or point this out in the output or docs (so that the next person with this problem doesn't have to dive into the code)?

jaapdeheer commented 1 month ago

With RAYON_NUM_THREADS=4 it seems to work fine.

I'm not so sure anymore about my theory of multiple biome instances running at the same time, since biome only takes like 1 second and we don't have that many concurrent CI runs. Anyway, I think we'll just set RAYON_NUM_THREADS and call it a day.

I would still suggest adding a hint to the error or docs, or defaulting to a lower max number of threads. Maybe on ThreadPoolBuildError, biome could not say "this is bug in biome, not in your code" but something more like "looks like you don't have enough resources for the thread pool, try setting RAYON_NUM_THREADS to something low"? My 2c :)

Thanks for your help @ematipico!

ematipico commented 1 month ago

That's definitely a good suggestion. I think I will create a new environment variable for CI environments that will alias the Rayon one