-
Notifications
You must be signed in to change notification settings - Fork 2.7k
feat(core): add logging and progress message types to daemon #35342
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
Changes from 4 commits
ecca529
2f94962
b12a456
a598127
0c31a66
a7bab9e
201e12a
7d19437
7cc774d
f64e9e8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| export const UPDATE_PROGRESS_MESSAGE = 'UPDATE_PROGRESS_MESSAGE' as const; | ||
|
|
||
| export type UpdateProgressMessage = { | ||
| type: typeof UPDATE_PROGRESS_MESSAGE; | ||
| message: string; | ||
| }; | ||
|
|
||
| export function isUpdateProgressMessage( | ||
| message: unknown | ||
| ): message is UpdateProgressMessage { | ||
| return ( | ||
| typeof message === 'object' && | ||
| message !== null && | ||
| 'type' in message && | ||
| message['type'] === UPDATE_PROGRESS_MESSAGE | ||
| ); | ||
| } | ||
|
|
||
| export const EMIT_LOG = 'EMIT_LOG' as const; | ||
|
|
||
| export type EmitLogLevel = 'log' | 'warn' | 'error'; | ||
|
|
||
| export type EmitLogMessage = { | ||
| type: typeof EMIT_LOG; | ||
| level: EmitLogLevel; | ||
| message: string; | ||
| }; | ||
|
|
||
| export function isEmitLogMessage(message: unknown): message is EmitLogMessage { | ||
| return ( | ||
| typeof message === 'object' && | ||
| message !== null && | ||
| 'type' in message && | ||
| message['type'] === EMIT_LOG | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| import { AsyncLocalStorage } from 'async_hooks'; | ||
| import type { Socket } from 'net'; | ||
| import { MESSAGE_END_SEQ } from '../../utils/consume-messages-from-socket'; | ||
| import { | ||
| EMIT_LOG, | ||
| EmitLogLevel, | ||
| UPDATE_PROGRESS_MESSAGE, | ||
| } from '../message-types/streaming-messages'; | ||
| import { isOnDaemon } from '../is-on-daemon'; | ||
| import { serverLogger } from '../logger'; | ||
|
|
||
| // Messages that stream back to the client while a request is in flight | ||
| // (progress updates, log forwarding) need to know which socket belongs to | ||
| // the currently-handled request. Threading a socket through every layer | ||
| // of the handler call tree would be invasive, so we stash it in | ||
| // AsyncLocalStorage and surface it via the helpers below. | ||
| const clientSocketStorage = new AsyncLocalStorage<Socket>(); | ||
|
|
||
| export function runWithClientSocket<T>( | ||
| socket: Socket, | ||
| fn: () => Promise<T> | T | ||
| ): Promise<T> | T { | ||
| return clientSocketStorage.run(socket, fn); | ||
| } | ||
|
|
||
| export function getActiveClientSocket(): Socket | undefined { | ||
| return clientSocketStorage.getStore(); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. inline this |
||
|
|
||
| function assertOnDaemon(helperName: string) { | ||
| if (!isOnDaemon()) { | ||
| throw new Error( | ||
| `${helperName} can only be called from the Nx daemon process.` | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| function writeStreamingMessage(socket: Socket, payload: unknown) { | ||
| try { | ||
| socket.write(JSON.stringify(payload) + MESSAGE_END_SEQ, (err) => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use the JSON serialize abstraction with v8 handling. |
||
| if (err) { | ||
| serverLogger.log( | ||
| `Streaming message write error (client likely disconnected): ${err.message}` | ||
| ); | ||
| } | ||
| }); | ||
| } catch (e) { | ||
| serverLogger.log( | ||
| `Failed to send streaming message to client: ${ | ||
| e instanceof Error ? e.message : String(e) | ||
| }` | ||
| ); | ||
| } | ||
|
Comment on lines
+67
to
+82
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Look at |
||
| } | ||
|
|
||
| /** | ||
| * Sends a progress message to the currently-connected client, which will | ||
| * update the in-flight spinner on the client side. No-op when called | ||
| * outside of a request-handling async context (no active client socket). | ||
| * | ||
| * Must only be invoked from inside the Nx daemon process. | ||
| */ | ||
| export function sendProgressMessageToClient(message: string): void { | ||
| assertOnDaemon('sendProgressMessageToClient'); | ||
| const socket = getActiveClientSocket(); | ||
| if (!socket) return; | ||
| writeStreamingMessage(socket, { | ||
| type: UPDATE_PROGRESS_MESSAGE, | ||
| message, | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Emits a log line to the currently-connected client. If invoked outside | ||
| * of a request-handling async context (e.g. background daemon work with | ||
| * no subscriber), the message is written to the daemon logger instead so | ||
| * it is not silently dropped. | ||
| * | ||
| * Must only be invoked from inside the Nx daemon process. | ||
| */ | ||
| export function emitLogToClient(level: EmitLogLevel, message: string): void { | ||
|
FrozenPandaz marked this conversation as resolved.
Outdated
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Proxy this function to the daemonLogger |
||
| assertOnDaemon('emitLogToClient'); | ||
| const socket = getActiveClientSocket(); | ||
| if (!socket) { | ||
| serverLogger.log(`[emit-log:${level}] ${message}`); | ||
| return; | ||
| } | ||
| writeStreamingMessage(socket, { | ||
| type: EMIT_LOG, | ||
| level, | ||
| message, | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,9 +29,18 @@ import type { | |
| MessageResult, | ||
| PluginWorkerLoadResult, | ||
| PluginWorkerMessage, | ||
| PluginWorkerNotification, | ||
| PluginWorkerResult, | ||
| } from './messaging'; | ||
| import { isPluginWorkerResult, sendMessageOverSocket } from './messaging'; | ||
| import { | ||
| isPluginWorkerNotification, | ||
| isPluginWorkerResult, | ||
| sendMessageOverSocket, | ||
| } from './messaging'; | ||
| import { | ||
| emitLogToClient, | ||
| sendProgressMessageToClient, | ||
| } from '../../../daemon/server/client-socket-context'; | ||
| import { | ||
| Hook, | ||
| Phase, | ||
|
|
@@ -213,6 +222,10 @@ export class IsolatedPlugin implements LoadedNxPlugin { | |
|
|
||
| private handleSocketData = (raw: string) => { | ||
| const message = parseMessage<any>(raw); | ||
| if (isPluginWorkerNotification(message)) { | ||
| handlePluginWorkerNotification(message); | ||
| return; | ||
| } | ||
| if (!isPluginWorkerResult(message)) { | ||
| return; | ||
| } | ||
|
|
@@ -696,3 +709,28 @@ type Falsy = false | 0 | '' | null | undefined | 0n; | |
| function hooks(...array: Array<Hook | Falsy>): Array<Hook> { | ||
| return array.filter((v): v is Hook => !!v); | ||
| } | ||
|
|
||
| // When the host process is the daemon, forward notifications to the | ||
| // currently-connected client so log lines surface in the user's | ||
| // terminal and progress updates reach the client-side spinner. When the | ||
| // host is the direct CLI there is no client socket, so log lines go | ||
| // straight to stdout/stderr; progress updates from workers have no | ||
| // destination in that case and are dropped (the CLI's own in-process | ||
| // plugin-loading loop is responsible for spinner updates there). | ||
| function handlePluginWorkerNotification( | ||
| notification: PluginWorkerNotification | ||
| ): void { | ||
| if ((global as any).NX_DAEMON) { | ||
| if (notification.type === 'emitLog') { | ||
| emitLogToClient(notification.level, notification.message); | ||
| } else if (notification.type === 'updateProgress') { | ||
| sendProgressMessageToClient(notification.message); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't currently have a use case for progress messages from plugin workers... so let's remove it for now. |
||
| } | ||
| return; | ||
| } | ||
| if (notification.type === 'emitLog') { | ||
| const stream = | ||
| notification.level === 'error' ? process.stderr : process.stdout; | ||
| stream.write(notification.message + '\n'); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import type { Socket } from 'net'; | ||
| import { | ||
| PluginWorkerEmitLogNotification, | ||
| PluginWorkerUpdateProgressNotification, | ||
| sendMessageOverSocket, | ||
| } from './messaging'; | ||
|
|
||
| // Plugin workers talk to their host process over a single socket that is | ||
| // established when the host connects. Plugin code running anywhere in the | ||
| // worker process needs a way to emit log lines / progress updates without | ||
| // having that socket threaded through every call frame, so we stash a | ||
| // module-level reference here when the host connects. | ||
| let hostSocket: Socket | null = null; | ||
|
|
||
| export function setPluginWorkerHostSocket(socket: Socket): void { | ||
| hostSocket = socket; | ||
| socket.once('close', () => { | ||
| if (hostSocket === socket) hostSocket = null; | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Emits a log line from the plugin worker up to its host. The host | ||
| * decides where it ends up (direct stdout/stderr when running under the | ||
| * CLI, forwarded to the active daemon client when running under the | ||
| * daemon). | ||
| * | ||
| * No-op when called outside of a plugin worker (i.e. no host connected). | ||
| */ | ||
| export function emitPluginWorkerLog( | ||
| level: PluginWorkerEmitLogNotification['level'], | ||
| message: string | ||
| ): void { | ||
| if (!hostSocket) return; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If plugin isolation is turned off this should probably log it? |
||
| sendMessageOverSocket(hostSocket, { | ||
| type: 'emitLog', | ||
| level, | ||
| message, | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Emits a progress message from the plugin worker up to its host. The | ||
| * host forwards it to the active daemon client (if running under the | ||
| * daemon) so an in-flight spinner can reflect the update. No-op when | ||
| * called outside of a plugin worker. | ||
| */ | ||
| export function emitPluginWorkerProgress(message: string): void { | ||
| if (!hostSocket) return; | ||
| sendMessageOverSocket(hostSocket, { | ||
| type: 'updateProgress', | ||
| message, | ||
| } satisfies PluginWorkerUpdateProgressNotification); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is disallowed