Skip to content

Commit 45fb65d

Browse files
fix(local): address review findings, remove learn-more line
- Close failed server on EADDRINUSE before retrying (prevents fd leak) - Fix SSE chunk boundary: use partial line buffer across chunks - Fix SSE data field: spec-compliant single-space strip instead of trimStart - Add logger.debug to isServerRunning catch block (repo policy) - Use process.once for signal handlers in consumer mode - Guard abort race in quiet consumer mode - Remove dead onEnvelope parameter from buildApp - Remove 'Learn more about Spotlight' line from startup banner
1 parent f411cf5 commit 45fb65d

1 file changed

Lines changed: 33 additions & 23 deletions

File tree

src/commands/local.ts

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,7 @@ const LOCALHOST_ORIGIN_RE = /^https?:\/\/(localhost|127\.0\.0\.1)(:\d+)?$/;
106106
* arbitrary remote origins to read the SSE envelope stream.
107107
*/
108108
function buildApp(
109-
spotlightBuffer: ReturnType<typeof createSpotlightBuffer>,
110-
onEnvelope?: (contentType: string, data: Buffer) => void
109+
spotlightBuffer: ReturnType<typeof createSpotlightBuffer>
111110
): Hono {
112111
const app = new Hono();
113112

@@ -157,18 +156,14 @@ function buildApp(
157156
| undefined;
158157
const userAgent = c.req.header("user-agent");
159158

160-
const container = pushToSpotlightBuffer({
159+
pushToSpotlightBuffer({
161160
spotlightBuffer,
162161
body,
163162
encoding: contentEncoding,
164163
contentType,
165164
userAgent,
166165
});
167166

168-
if (container) {
169-
onEnvelope?.(container.getContentType(), container.getData());
170-
}
171-
172167
return c.body(null, 204);
173168
};
174169

@@ -640,6 +635,7 @@ function tryListen(
640635

641636
server.once("listening", () => resolve({ server, port }));
642637
server.once("error", async (err: NodeJS.ErrnoException) => {
638+
server.close();
643639
if (err.code === "EADDRINUSE") {
644640
attempts += 1;
645641
if (attempts > MAX_PORT_RETRIES) {
@@ -673,7 +669,11 @@ async function isServerRunning(url: string): Promise<boolean> {
673669
try {
674670
const res = await fetch(`${url}/health`);
675671
return res.ok;
676-
} catch {
672+
} catch (err) {
673+
logger.debug(
674+
`No existing server at ${url}`,
675+
err instanceof Error ? err.message : String(err)
676+
);
677677
return false;
678678
}
679679
}
@@ -691,9 +691,11 @@ function feedSSELine(
691691
onEvent: (type: string, data: string) => void
692692
): void {
693693
if (line.startsWith("event:")) {
694-
state.eventType = line.slice(6).trim();
694+
const value = line.slice(6);
695+
state.eventType = value.startsWith(" ") ? value.slice(1) : value;
695696
} else if (line.startsWith("data:")) {
696-
state.dataLines.push(line.slice(5).trimStart());
697+
const value = line.slice(5);
698+
state.dataLines.push(value.startsWith(" ") ? value.slice(1) : value);
697699
} else if (line === "" && state.dataLines.length > 0) {
698700
onEvent(state.eventType, state.dataLines.join("\n"));
699701
state.eventType = "";
@@ -722,17 +724,25 @@ async function consumeSSE(
722724

723725
const decoder = new TextDecoder();
724726
const state: SSEParserState = { eventType: "", dataLines: [] };
727+
const onEvent = (type: string, data: string) => {
728+
if (type === SENTRY_CONTENT_TYPE) {
729+
processSSEEvent(data, activeFilters);
730+
}
731+
};
725732

733+
let partial = "";
726734
for await (const chunk of res.body) {
727-
const text = decoder.decode(chunk as Uint8Array, { stream: true });
728-
for (const rawLine of text.split("\n")) {
729-
feedSSELine(rawLine.replace(CR_RE, ""), state, (type, data) => {
730-
if (type === SENTRY_CONTENT_TYPE) {
731-
processSSEEvent(data, activeFilters);
732-
}
733-
});
735+
const text =
736+
partial + decoder.decode(chunk as Uint8Array, { stream: true });
737+
const lines = text.split("\n");
738+
partial = lines.pop() ?? "";
739+
for (const rawLine of lines) {
740+
feedSSELine(rawLine.replace(CR_RE, ""), state, onEvent);
734741
}
735742
}
743+
if (partial) {
744+
feedSSELine(partial.replace(CR_RE, ""), state, onEvent);
745+
}
736746
}
737747

738748
/** Parse and format a single SSE data payload from upstream. */
@@ -777,7 +787,6 @@ export const localCommand = buildCommand({
777787
"If a server is already listening on the port, the command connects\n" +
778788
"as an SSE consumer and tails events from it. Otherwise it starts\n" +
779789
"its own server.\n\n" +
780-
"Learn more: https://spotlightjs.com/docs/getting-started/\n\n" +
781790
"Press Ctrl-C to stop.",
782791
},
783792
parameters: {
@@ -829,11 +838,15 @@ export const localCommand = buildCommand({
829838

830839
const ac = new AbortController();
831840
const stop = () => ac.abort();
832-
process.on("SIGINT", stop);
833-
process.on("SIGTERM", stop);
841+
process.once("SIGINT", stop);
842+
process.once("SIGTERM", stop);
834843

835844
if (flags.quiet) {
836845
await new Promise<void>((resolve) => {
846+
if (ac.signal.aborted) {
847+
resolve();
848+
return;
849+
}
837850
ac.signal.addEventListener("abort", () => resolve());
838851
});
839852
} else {
@@ -872,9 +885,6 @@ export const localCommand = buildCommand({
872885
if (activeFilters.size > 0) {
873886
logger.info(`Filtering: ${[...activeFilters].join(", ")}`);
874887
}
875-
logger.info(
876-
`Learn more about Spotlight: ${bold("https://spotlightjs.com/docs/getting-started/")}`
877-
);
878888
logger.info("Press Ctrl-C to stop.");
879889

880890
await waitForShutdown(server);

0 commit comments

Comments
 (0)