jason810496 commented on code in PR #69302:
URL: https://github.com/apache/airflow/pull/69302#discussion_r3568422995
##########
ts-sdk/src/coordinator/log-channel.ts:
##########
@@ -47,31 +47,45 @@ export interface LogRecord {
const DEFAULT_LOGGER_NAME = "ts-sdk";
+interface LogChannelState {
+ sock: Socket;
+ connected: boolean;
+ closed: boolean;
+}
+
export class LogChannel {
- private readonly sock: Socket;
+ private readonly shared: LogChannelState;
private readonly name: string;
private readonly isRoot: boolean;
- private constructor(sock: Socket, name: string, isRoot: boolean) {
- this.sock = sock;
+ private constructor(shared: LogChannelState, name: string, isRoot: boolean) {
+ this.shared = shared;
this.name = name;
this.isRoot = isRoot;
- if (isRoot) {
- sock.on("error", (err) => {
- process.stderr.write(`[${this.name}] log socket error:
${err.message}\n`);
- });
- }
}
static async connect(addr: string, name: string = DEFAULT_LOGGER_NAME):
Promise<LogChannel> {
- return new LogChannel(await connectTcp(addr), name, true);
+ const shared: LogChannelState = {
+ sock: await connectTcp(addr),
+ connected: true,
+ closed: false,
+ };
+ shared.sock.on("error", (err) => {
+ process.stderr.write(`[${name}] log socket error: ${err.message}\n`);
+ });
Review Comment:
Caught by Claude:
The `"error"` handler only writes to stderr; only the later `"close"`
handler flips `shared.connected = false`. Node fires `"close"` a tick or so
after `"error"` (empirically ~0.05ms later in a local repro), and in that gap a
`send()` call still takes the "connected" branch and writes to an
already-destroyed socket — the record is silently dropped instead of falling
back to stderr as intended. Setting the flag in the `error` handler too closes
the gap:
```suggestion
shared.sock.on("error", (err) => {
shared.connected = false;
process.stderr.write(`[${name}] log socket error: ${err.message}\n`);
});
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
