Skip to content

Commit b709cc2

Browse files
1stvamp1stvamp
committed
fix(observability-map): remove the last dead scanner field and pin the callee-path fallback
CatchEvidence.narrow was the same species as the fields removed earlier: written by scan.ts, asserted in tests, read by nothing. error-classification computes narrowness itself inline through isParseGuard and never reads clause.narrow. Removed the field, its NARROW_TRY_STATEMENTS derivation, and every test asserting it; kept tryStatementCount, which isParseGuard reads. Also pins the two callee-path shapes the earlier callee-texts test deletion left unexercised: a three-level property chain (prisma.organization.findFirst arriving as findFirst in calleeNames) and the constructor-fallback shape (new PromptService().createOverride), plus a logger call in the same fixture to confirm calleeText's normal chain-building still feeds LogCall.callee.
1 parent 30cd989 commit b709cc2

4 files changed

Lines changed: 56 additions & 192 deletions

File tree

‎internal-packages/observability-map/src/checks/errorClassification.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,10 @@ function isParseGuard(clause: CatchEvidence, ep: EntryPoint): boolean {
5454
* `request-context` still reads that log and asks whether it names a tenant, so the reporting is
5555
* unrewarded here rather than unmeasured.
5656
*
57-
* `narrow` is not a way to qualify either. A one-statement try around `await service.call(run)` is
58-
* narrow and is still a swallow: reading all eleven entry points that limb would clear said six
59-
* were real, including a silent run cancellation and two credential paths that report a database
60-
* failure to the browser as a 400 with an internal message in it.
57+
* A narrow guard is not a way to qualify either. A one-statement try around `await
58+
* service.call(run)` is narrow and is still a swallow: reading all eleven entry points that limb
59+
* would clear said six were real, including a silent run cancellation and two credential paths
60+
* that report a database failure to the browser as a 400 with an internal message in it.
6161
*/
6262
function decides(clause: CatchEvidence, ep: EntryPoint): boolean {
6363
return clause.branches || isParseGuard(clause, ep);

‎internal-packages/observability-map/src/scan.ts‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -107,13 +107,6 @@ function objectArgumentFields(call: ts.CallExpression): string[] {
107107
return [];
108108
}
109109

110-
/**
111-
* How much a try block may guard and still count as narrow. Two, so that the guarded operation can
112-
* bind its result (`const stripped = ...; new RegExp(stripped);`) but a third statement means the
113-
* try has started to cover the handler rather than one operation.
114-
*/
115-
const NARROW_TRY_STATEMENTS = 2;
116-
117110
/**
118111
* Calls that turn input into a value and throw when it is malformed. `parse`/`safeParse` cover
119112
* `JSON.parse` and the zod schemas. `.json` has to be a member call, because a bare `json(...)` is
@@ -563,7 +556,6 @@ export function scanFile(fileName: string, source: string): EntryPoint | null {
563556
const tryStatementCount = countStatements(node.tryBlock.statements);
564557
const clause = catchClauseEvidence(node.catchClause);
565558
catches.push({
566-
narrow: tryStatementCount <= NARROW_TRY_STATEMENTS,
567559
rethrows: clause.rethrows,
568560
branches: clause.branches,
569561
guardsParse: guardsParse(node.tryBlock),

‎internal-packages/observability-map/src/types.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ export type CheckResult = {
1212
* legible instead of collapsing into one boolean.
1313
*/
1414
export type CatchEvidence = {
15-
/** The guarded try block holds at most two statements: one operation, not the handler. */
16-
narrow: boolean;
1715
/** The clause contains a `throw`. */
1816
rethrows: boolean;
1917
/**

‎internal-packages/observability-map/test/scan.test.ts‎

Lines changed: 52 additions & 178 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,31 @@ describe("scanFile: callee resolution", () => {
460460
expect(ep!.hasLoader).toBe(true);
461461
expect(ep!.loaderInitializerCallee).toBeNull();
462462
});
463+
464+
it("resolves a multi-level call and falls back to the bare name past an unnameable one", () => {
465+
const ep = scanFile(
466+
"api.v1.things.ts",
467+
`
468+
export async function loader({ request }) {
469+
try {
470+
const org = await prisma.organization.findFirst({ where: { id: 1 } });
471+
return json(await new PromptService().createOverride(org));
472+
} catch (e) {
473+
logger.error("nope", { error: e });
474+
return json({}, { status: 500 });
475+
}
476+
}
477+
`
478+
);
479+
// A three-level property chain still lands on its bare method name.
480+
expect(ep!.calleeNames).toContain("findFirst");
481+
// A chain through a `new` expression has no name of its own, so this also falls back to the
482+
// bare name rather than losing the call.
483+
expect(ep!.calleeNames).toContain("createOverride");
484+
// The full path still builds where nothing unnameable sits in it, which is what `LogCall.callee`
485+
// depends on.
486+
expect(ep!.logCalls[0]!.callee).toBe("logger.error");
487+
});
463488
});
464489

465490
describe("scanFile: parse failures", () => {
@@ -724,62 +749,7 @@ describe("scanFile: catch clause evidence", () => {
724749
expect(ep!.hasTryCatch).toBe(true);
725750
expect(ep!.catches).toEqual([]);
726751
});
727-
});
728752

729-
describe("scanFile: log calls", () => {
730-
it("records the fields of a log call's object argument", () => {
731-
const ep = scanFile(
732-
"logging.ts",
733-
`
734-
export async function loader({ request }) {
735-
try {
736-
return json(await load(request));
737-
} catch (e) {
738-
logger.error("load failed", { environmentId: env.id, error: e });
739-
return json({}, { status: 500 });
740-
}
741-
}
742-
`
743-
);
744-
expect(ep!.logCalls).toHaveLength(1);
745-
expect(ep!.logCalls[0]).toEqual({
746-
callee: "logger.error",
747-
fields: ["environmentId", "error"],
748-
inCatch: true,
749-
});
750-
});
751-
752-
it("records a log call with no object argument, outside a catch", () => {
753-
const ep = scanFile(
754-
"logging-plain.ts",
755-
`
756-
export async function loader() {
757-
log.info("starting");
758-
return json({});
759-
}
760-
`
761-
);
762-
expect(ep!.logCalls).toEqual([{ callee: "log.info", fields: [], inCatch: false }]);
763-
});
764-
765-
it("ignores a non-logger call and a log call in the React component", () => {
766-
const ep = scanFile(
767-
"route.tsx",
768-
`
769-
export async function loader() {
770-
return json(await load());
771-
}
772-
export default function Page() {
773-
logger.debug("rendered", { runId: 1 });
774-
return null;
775-
}
776-
`
777-
);
778-
expect(ep!.logCalls).toEqual([]);
779-
});
780-
});
781-
782-
describe("scanFile: narrow catches", () => {
783753
it("flags a try that guards a single request.json()", () => {
784754
const ep = scanFile(
785755
"admin.api.v1.platform-notifications.ts",
@@ -800,161 +770,70 @@ describe("scanFile: narrow catches", () => {
800770
expect(ep!.hasTryCatch).toBe(true);
801771
expect(ep!.catches[0]!.rethrows).toBe(false);
802772
expect(ep!.catches[0]!.branches).toBe(false);
803-
expect(ep!.catches[0]!.narrow).toBe(true);
804-
});
805-
806-
it("does not flag a catch wrapping the whole handler", () => {
807-
const ep = scanFile(
808-
"otel.v1.logs.ts",
809-
`
810-
export async function action({ request }) {
811-
try {
812-
const exporter = await otlpExporter;
813-
const contentType = request.headers.get("content-type") ?? "";
814-
const body = await request.json();
815-
await exporter.export(body);
816-
return json({ ok: true });
817-
} catch (e) {
818-
logger.error(e);
819-
return json({}, { status: 500 });
820-
}
821-
}
822-
`
823-
);
824-
expect(ep!.hasTryCatch).toBe(true);
825-
expect(ep!.catches[0]!.narrow).toBe(false);
826-
});
827-
828-
it("keeps a narrow catch and a broad one distinct", () => {
829-
const ep = scanFile(
830-
"mixed.ts",
831-
`
832-
export async function action({ request }) {
833-
let body;
834-
try {
835-
body = await request.json();
836-
} catch {
837-
return json({}, { status: 400 });
838-
}
839-
try {
840-
const run = await find(body.id);
841-
const updated = await update(run);
842-
await notify(updated);
843-
return json(updated);
844-
} catch (e) {
845-
return json({}, { status: 500 });
846-
}
847-
}
848-
`
849-
);
850-
expect(ep!.catches[0]!.narrow).toBe(true);
851-
expect(ep!.catches[1]!.narrow).toBe(false);
852-
});
853-
854-
it("allows a guarded operation with its own local binding", () => {
855-
const ep = scanFile(
856-
"regex.ts",
857-
`
858-
export async function action({ request }) {
859-
const pattern = await patternFrom(request);
860-
try {
861-
const stripped = pattern.startsWith("(?i)") ? pattern.slice(4) : pattern;
862-
new RegExp(stripped);
863-
} catch {
864-
return json({ error: "Invalid regex" }, { status: 400 });
865-
}
866-
return json({ ok: true });
867-
}
868-
`
869-
);
870-
expect(ep!.catches[0]!.narrow).toBe(true);
871-
});
872-
873-
it("does not flag a try of three statements", () => {
874-
const ep = scanFile(
875-
"three.ts",
876-
`
877-
export async function loader({ request }) {
878-
try {
879-
const raw = await request.json();
880-
const parsed = Schema.parse(raw);
881-
return json(parsed);
882-
} catch {
883-
return json({}, { status: 400 });
884-
}
885-
}
886-
`
887-
);
888-
expect(ep!.catches[0]!.narrow).toBe(false);
889773
});
890774

891775
it("is empty when there is no try at all", () => {
892776
const ep = scanFile("plain.ts", `export async function loader() { return json({}); }`);
893777
expect(ep!.hasTryCatch).toBe(false);
894778
expect(ep!.catches).toEqual([]);
895779
});
780+
});
896781

897-
it("is empty for a try with a finally and no catch", () => {
782+
describe("scanFile: log calls", () => {
783+
it("records the fields of a log call's object argument", () => {
898784
const ep = scanFile(
899-
"finally-only.ts",
785+
"logging.ts",
900786
`
901-
export async function loader() {
787+
export async function loader({ request }) {
902788
try {
903-
return json(await load());
904-
} finally {
905-
release();
789+
return json(await load(request));
790+
} catch (e) {
791+
logger.error("load failed", { environmentId: env.id, error: e });
792+
return json({}, { status: 500 });
906793
}
907794
}
908795
`
909796
);
910-
expect(ep!.hasTryCatch).toBe(true);
911-
expect(ep!.catches).toEqual([]);
797+
expect(ep!.logCalls).toHaveLength(1);
798+
expect(ep!.logCalls[0]).toEqual({
799+
callee: "logger.error",
800+
fields: ["environmentId", "error"],
801+
inCatch: true,
802+
});
912803
});
913804

914-
it("reads a narrow catch inside a same-file helper the body delegates to", () => {
805+
it("records a log call with no object argument, outside a catch", () => {
915806
const ep = scanFile(
916-
"helper-narrow.ts",
807+
"logging-plain.ts",
917808
`
918-
function parseTags(payload) {
919-
try {
920-
return JSON.parse(payload);
921-
} catch {
922-
return null;
923-
}
924-
}
925-
export async function loader({ params }) {
926-
return json(parseTags(params.payload));
809+
export async function loader() {
810+
log.info("starting");
811+
return json({});
927812
}
928813
`
929814
);
930-
expect(ep!.hasTryCatch).toBe(true);
931-
expect(ep!.catches[0]!.narrow).toBe(true);
815+
expect(ep!.logCalls).toEqual([{ callee: "log.info", fields: [], inCatch: false }]);
932816
});
933817

934-
it("ignores a narrow catch that lives in the React component", () => {
818+
it("ignores a non-logger call and a log call in the React component", () => {
935819
const ep = scanFile(
936820
"route.tsx",
937821
`
938822
export async function loader() {
939-
return json({});
823+
return json(await load());
940824
}
941825
export default function Page() {
942-
try {
943-
JSON.parse(raw);
944-
} catch {
945-
return null;
946-
}
826+
logger.debug("rendered", { runId: 1 });
947827
return null;
948828
}
949829
`
950830
);
951-
expect(ep!.hasTryCatch).toBe(false);
952-
expect(ep!.catches).toEqual([]);
831+
expect(ep!.logCalls).toEqual([]);
953832
});
954833
});
955834

956835
describe("scanFile: per-catch evidence", () => {
957-
it("records one entry per catch clause, keeping a narrow guard distinct from a broad catch", () => {
836+
it("records one entry per catch clause, keeping a parse guard distinct from a broad catch", () => {
958837
const ep = scanFile(
959838
"two-catches.ts",
960839
`
@@ -978,14 +857,12 @@ describe("scanFile: per-catch evidence", () => {
978857
);
979858
expect(ep!.catches).toHaveLength(2);
980859
expect(ep!.catches[0]).toEqual({
981-
narrow: true,
982860
rethrows: false,
983861
branches: false,
984862
guardsParse: true,
985863
tryStatementCount: 1,
986864
});
987865
expect(ep!.catches[1]).toMatchObject({
988-
narrow: false,
989866
guardsParse: false,
990867
tryStatementCount: 4,
991868
});
@@ -1035,10 +912,9 @@ describe("scanFile: per-catch evidence", () => {
1035912
);
1036913
expect(ep!.catches).toHaveLength(1);
1037914
expect(ep!.catches[0]!.guardsParse).toBe(true);
1038-
expect(ep!.catches[0]!.narrow).toBe(true);
1039915
});
1040916

1041-
it("sees a parse in a try that grew past the narrowness threshold", () => {
917+
it("sees a parse in a try that outgrows a single statement", () => {
1042918
const ep = scanFile(
1043919
"admin.api.v1.orgs.$organizationId.stream-basin.ts",
1044920
`
@@ -1060,7 +936,6 @@ describe("scanFile: per-catch evidence", () => {
1060936
`
1061937
);
1062938
expect(ep!.catches).toHaveLength(1);
1063-
expect(ep!.catches[0]!.narrow).toBe(false);
1064939
expect(ep!.catches[0]!.guardsParse).toBe(true);
1065940
expect(ep!.catches[0]!.tryStatementCount).toBe(6);
1066941
});
@@ -1083,7 +958,6 @@ describe("scanFile: per-catch evidence", () => {
1083958
);
1084959
expect(ep!.catches).toHaveLength(1);
1085960
expect(ep!.catches[0]).toEqual({
1086-
narrow: false,
1087961
rethrows: false,
1088962
branches: false,
1089963
guardsParse: false,

0 commit comments

Comments
 (0)