Skip to content

Simpler default reporter proposal - #967

Merged
Danny van Velzen (dannyvv) merged 17 commits into
microsoft:masterfrom
crhaglun:master
Dec 18, 2025
Merged

Simpler default reporter proposal#967
Danny van Velzen (dannyvv) merged 17 commits into
microsoft:masterfrom
crhaglun:master

Conversation

@crhaglun

@crhaglun Christoffer Haglund (crhaglun) commented Nov 28, 2025

Copy link
Copy Markdown
Contributor

This is a proposal for a new default logger with a lower update frequency than ProgressReporter.

My team uses lage with a reasonably large monorepo. The default developer setup is a cloud devbox, running builds from the Visual Studio Code terminal window. A typical scoped build for our team covers about 300 projects out of 2000.

We noticed an excessive amount of time is spent by the VS Code UI thread; using less pretty reporters like "npmLog" or "json" actually reduces the build time by about 40% for a no-cache build (from 25 minutes to 15 minutes), but even a fully cached build sees about 20% improvement (from 1 minute 40 seconds to 1 minute 20 seconds).

However, these reporters does not have any progress indication which makes them less suitable for an interactive console.

This PR switches ProgressReporter, which is based on @ms-cloudpack/task-reporter, for a simpler reporter, but still using a progress indicator. Effectively, the rendering update increases from 80ms to 500ms.

Testing this approach in the real monorepo shows equivalent build improvements as using non-interactive reporters, i.e. 20% to 40% build time improvement depending on the state of the repo. Even a super small repo like lage itself consistently shows a small but consistent improvement using BasicReporter compared to ProgressReporter.

Screen capture building the lage repo from the fork this PR comes from, using the worst-performing terminal on my machine:
lage-build-success

Build errors are reported similarly to the original ProgressReporter:
image

Other issues uncovered and fixed while working on this

  • Concurrency bug in ChromeTraceEventsReporter; we open the output stream early and use logStream.write() to write output. Flushing of data is potentially an async operation under the hood in Node, so when the test aggressively delete the temp folder in "afterAll", we occasionally see an out-of-order write error. This causes a test failure which gets attributed to the wrong test.
  • There is no longer a "--safe-exit" flag in Lage v2, the unit test for this flag is effectively identical to the general build failure test.

Christoffer Haglund added 15 commits November 28, 2025 12:09
…s microseconds, not milliseconds. Focus on simplicity instead.
…gle line for progress information, use ansi codes directly to print completion message + new status line in one call.
When running tests, we occasionally see intermittent failures like
````
node:events:497
      throw er; // Unhandled 'error' event
      ^

Error: ENOENT: no such file or directory, open '/tmp/lage-B8VSbk/profile.json'
Emitted 'error' event on WriteStream instance at:
    at emitErrorNT (node:internal/streams/destroy:170:8)
    at emitErrorCloseNT (node:internal/streams/destroy:129:3)
    at processTicksAndRejections (node:internal/process/task_queues:90:21) {
  errno: -2,
  code: 'ENOENT',
  syscall: 'open',
  path: '/tmp/lage-B8VSbk/profile.json'
}
````

In ChromeTraceEventsReporter, we open the target file stream in the object constructor and write events with logStream.write(), counting on the nodje cleanup to close file descriptors.

But, in initializeReporters.test.ts, we create a temp folder in `beforeAll` and delete it in `afterAll`.

`write` does not flush buffers, this is done asynchronously by the Node runtime.

Hypothesis: Since we don't wait for buffers to be flushed, we sometimes clean up the temp folder before the last native write, causing an asynchronous error after the "should initialize profile reporter" test has already passed.

This happens out of order, which would explain why "random" tests fail.

ChromeTraceEventsReporterOptions writes data once, in the summary method.
Let's use the Node facility writeFileSync() to do a one-shot write which does not leave a dangling file handle.
…lure happens in `--safe-exit`, be sure to have exit code of !== 0" is redundant as it's identical to "when a failure happens be sure to have exit code of !== 0"
@dannyvv
Danny van Velzen (dannyvv) merged commit 209d31d into microsoft:master Dec 18, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants