feat(core): add logging and progress message types to daemon#35342
feat(core): add logging and progress message types to daemon#35342AgentEnder merged 10 commits intomasterfrom
Conversation
Introduces two new daemon message types that are not expected to resolve the pending client request promise: - UPDATE_PROGRESS_MESSAGE: updates the client-side spinner while a request is in flight so long-running work (e.g. plugin loading) can surface which step it is currently on. - EMIT_LOG: forwards a log line from the daemon to the requesting client so warnings emitted inside the daemon reach the user's terminal instead of disappearing into the daemon log file. The daemon wraps each handler in an AsyncLocalStorage-scoped context keyed on the requesting socket so code deep in the handler call tree can stream messages back to the right client without threading the socket through every layer. The client-side handleMessage is taught to route the new message types into globalSpinner.updateText and console[level] instead of resolving the pending promise. https://claude.ai/code/session_014sDuF5eLunmQPHsRRWLTMo
Plugin workers gain an emitLog / updateProgress notification channel back to their host process. Notifications are unsolicited (no tx, no response expected) so plugin code can fire them without coupling to the request/response protocol. The isolated-plugin host handles notifications differently depending on whose process it is running in: - When the host is the daemon, the notification is forwarded to the active client socket via the emit-log / update-progress-message streaming messages introduced in the previous commit. - When the host is the direct CLI (no daemon), log notifications are written straight to stdout/stderr. Progress notifications are dropped since the CLI already owns the in-process spinner. https://claude.ai/code/session_014sDuF5eLunmQPHsRRWLTMo
Wires up the first two real uses of the streaming message channel: - createProjectConfigurationsWithPlugins now sends its in-flight spinner text through sendProgressMessageToClient. Under the daemon the local spinner is a no-op, so this makes the client-side spinner reflect which plugin is still running (e.g. "Creating project graph nodes with @nx/jest") rather than the generic "Calculating the project graph on the Nx Daemon" message. - The graph cache-write failure warning in writeCache is routed to emitLogToClient when running on the daemon. Previously the warning landed only in the daemon log and users never saw it. https://claude.ai/code/session_014sDuF5eLunmQPHsRRWLTMo
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for nx-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
View your CI Pipeline Execution ↗ for commit f64e9e8
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Let's revert this one, its fine to stay in daemon logs
| // When running inside the daemon the local spinner is a no-op | ||
| // (daemon has no TTY), so forward the message to the connected | ||
| // client so their spinner reflects which plugin is still running. | ||
| sendProgressMessageToClient(message); |
There was a problem hiding this comment.
This should probably have a isOnDaemon guard
Addresses review feedback on #35342: - sendProgressMessageToClient and emitLogToClient now throw when called outside of the Nx daemon process, making misuse loud rather than silent. - createProjectConfigurationsWithPlugins gates the progress forwarding behind isOnDaemon() so the helpers are never reached in the direct CLI case. - Reverts the emit-log use of the cache-write failure warning — that warning is fine staying in the daemon log. https://claude.ai/code/session_014sDuF5eLunmQPHsRRWLTMo
| return; | ||
| } | ||
| if (isEmitLogMessage(parsedResult)) { | ||
| // eslint-disable-next-line no-console |
There was a problem hiding this comment.
I don't think this is disallowed
|
|
||
| function writeStreamingMessage(socket: Socket, payload: unknown) { | ||
| try { | ||
| socket.write(JSON.stringify(payload) + MESSAGE_END_SEQ, (err) => { |
There was a problem hiding this comment.
Use the JSON serialize abstraction with v8 handling.
| level: PluginWorkerEmitLogNotification['level'], | ||
| message: string | ||
| ): void { | ||
| if (!hostSocket) return; |
There was a problem hiding this comment.
If plugin isolation is turned off this should probably log it?
| export function getActiveClientSocket(): Socket | undefined { | ||
| return clientSocketStorage.getStore(); | ||
| } |
| if (isOnDaemon()) { | ||
| sendProgressMessageToClient(initialMessage); | ||
| } |
There was a problem hiding this comment.
Move this into DelayedSpinner and don't change this at all. This will make it work for createDependencies as well.
| * | ||
| * Must only be invoked from inside the Nx daemon process. | ||
| */ | ||
| export function emitLogToClient(level: EmitLogLevel, message: string): void { |
There was a problem hiding this comment.
Proxy this function to the daemonLogger
| if (notification.type === 'emitLog') { | ||
| emitLogToClient(notification.level, notification.message); | ||
| } else if (notification.type === 'updateProgress') { | ||
| sendProgressMessageToClient(notification.message); |
There was a problem hiding this comment.
We don't currently have a use case for progress messages from plugin workers... so let's remove it for now.
Addresses a second round of review feedback on #35342: - emitLogToClient is now a method on DaemonLogger (serverLogger.emitToClient) so daemon-to-client log forwarding lives alongside the rest of the daemon logging surface. - Progress forwarding moves into DelayedSpinner itself rather than living at the call site. Any caller that uses DelayedSpinner inside the daemon now automatically streams progress to the client — this picks up createDependencies and createMetadata for free. project-configuration-utils.ts is reverted to its original shape. - writeStreamingMessage uses the shared serialize() helper with v8 handling instead of a bespoke JSON.stringify call. - getActiveClientSocket is used across files, kept as a small named export rather than inlined. - emit-log notifications from plugin workers now fall back to stdout/stderr when no host socket is connected, so they aren't silently dropped when plugin isolation is off. - Drop the unused plugin-worker updateProgress notification path and accompanying types — there's no caller for it yet. - Remove an unnecessary eslint-disable on a console[level] call that isn't actually disallowed. https://claude.ai/code/session_014sDuF5eLunmQPHsRRWLTMo
Replace the AsyncLocalStorage single-socket progress model with a topic-based subscriber registry. Long-running daemon operations register subscribers for a named topic on entry and unregister on exit; broadcast helpers fan out to every currently-subscribed socket. - Introduces ProgressTopics.GraphConstruction as the first (and only) topic today. Add more as other daemon operations grow their own streaming surfaces. - getCachedSerializedProjectGraphPromise now takes the requesting socket and subscribes it to GraphConstruction for the duration of the await. Clients that join a recomputation already in flight now receive the remaining progress messages — previously only the client that initiated the compute got them, and file-watcher-triggered recomputations produced no notifications at all. - DelayedSpinner accepts an optional progressTopic option; when set and running inside the daemon, every setMessage broadcasts to that topic's subscribers. - DaemonLogger#logToClient now takes a topic and falls back to the daemon log when no clients are subscribed. https://claude.ai/code
The topic constant is the contract between progress producers (DelayedSpinner callers) and the daemon subscriber registry, so it belongs next to the spinner rather than inside the daemon's client-socket bookkeeping. Also sidesteps a latent import cycle between delayed-spinner.ts and the daemon server module. https://claude.ai/code
The client does not render hashing progress (handleMessage ignores streaming messages outside of REQUEST_PROJECT_GRAPH), so subscribing the hash path was pure coupling and wasted socket traffic. Revert handleHashTasks to its socket-less signature. https://claude.ai/code
The client previously mutated the process-wide globalSpinner on every incoming progress message, which could overwrite the text of an unrelated command that had started its own spinner. Since the daemon client is a serialized message queue, carry the owning spinner alongside the in-flight request via a new currentSpinner field (set inside the queued function and cleared in a finally). handleMessage now drives that spinner directly, so streamed updates never reach anyone else's UI. https://claude.ai/code
| private async sendToDaemonViaQueue<T extends DaemonMessage>( | ||
| messageToDaemon: T, | ||
| force?: 'v8' | 'json' | ||
| options?: { force?: 'v8' | 'json'; spinner?: DelayedSpinner } |
There was a problem hiding this comment.
| options?: { force?: 'v8' | 'json'; spinner?: DelayedSpinner } | |
| options?: { parser?: 'v8' | 'json'; spinner?: DelayedSpinner } |
| preventRecursionInGraphConstruction(); | ||
| let spinner: DelayedSpinner; | ||
| // If the graph takes a while to load, we want to show a spinner. | ||
| spinner = new DelayedSpinner( |
There was a problem hiding this comment.
Set this.currentSpinner here and add a finally block to null it.
| try { | ||
| socket.write(serialize(payload) + MESSAGE_END_SEQ, (err) => { | ||
| if (err) { | ||
| console.log( | ||
| `Streaming message write error (client likely disconnected): ${err.message}` | ||
| ); | ||
| } | ||
| }); | ||
| } catch (e) { | ||
| console.log( | ||
| `Failed to send streaming message to client: ${ | ||
| e instanceof Error ? e.message : String(e) | ||
| }` | ||
| ); | ||
| } |
There was a problem hiding this comment.
Look at respondToClient from shutdown-utils and borrow some ideas like logging this into the server logs etc.
| message: string | ||
| ): void { | ||
| assertOnDaemon('sendProgressMessageToTopic'); | ||
| const subscribers = topicSubscribers.get(topic); |
There was a problem hiding this comment.
Use getTopicSubscribers
| * | ||
| * Must only be invoked from inside the Nx daemon process. | ||
| */ | ||
| logToClient(topic: ProgressTopic, level: EmitLogLevel, message: string) { |
There was a problem hiding this comment.
This should import a function from the client socket context... shouldn't have the implementation here
| const stream = | ||
| notification.level === 'error' ? process.stderr : process.stdout; | ||
| stream.write(notification.message + '\n'); |
There was a problem hiding this comment.
Use console so that it is parity with the daemon client.
| const stream = level === 'error' ? process.stderr : process.stdout; | ||
| stream.write(message + '\n'); |
Addresses review feedback on #35342: - writeStreamingMessage now mirrors the logging shape of respondToClient in shutdown-utils (broadcast / done / error lines via serverLogger, with the message type as the description). - Move the broadcast-to-subscribers implementation into client-socket-context as sendLogToTopic; DaemonLogger#logToClient now orchestrates it and falls back to the daemon log only when nobody is subscribed. - sendProgressMessageToTopic and sendLogToTopic go through the exported getTopicSubscribers helper rather than peeking at the internal map directly. - Rename the sendToDaemonViaQueue option from `force` to `parser` for clarity and drop the spinner plumbing from it. The caller (getProjectGraphAndSourceMaps) now sets `this.currentSpinner` directly and clears it in a finally block. - isolated-plugin.ts and worker-streaming.ts fall back to `console[level]()` for log notifications instead of raw process.stdout/stderr writes, keeping parity with how the daemon client renders emit-log messages. https://claude.ai/code/session_014sDuF5eLunmQPHsRRWLTMo
f3405d9 to
8a1c4ba
Compare
| function updateSpinner() { | ||
| function getSpinnerText() { | ||
| if (!spinner || inProgressPlugins.size === 0) { | ||
| return; |
There was a problem hiding this comment.
Does this clear the spinner message?
| function getSpinnerText() { | ||
| if (!spinner || inProgressPlugins.size === 0) { | ||
| return; | ||
| return ''; |
There was a problem hiding this comment.
Is this correct or is the one above correct?
8a1c4ba to
f64e9e8
Compare
|
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
Current Behavior
During graph creation theres no simple way to know which plugin is taking time if the daemon is enabled. Instead, we instruct users to rerun with --no-daemon to see better logging..... which isnt ideal.
Expected Behavior
The daemon is capable of updating the client on its progress without triggering the current promises resolution / rejection
Related Issue(s)
Fixes #