Skip to content

Commit 0fdf03c

Browse files
committed
fix(observability-map): credit a thrown ternary that classifies the error
catch (e) { throw e instanceof Response ? e : new ServerError(e); } read as inert, while the same clause written with return passed. The walk set rethrows and cut the path before the shared branch check ran, so the throw arm of that condition was unreachable and a thrown conditional was never offered to selectsAnErrorPath. That contradicted the CatchEvidence.branches contract in types.ts, which names return and throw alike. The check now runs in the throw arm, before the path is cut, through the same selectsAnErrorPath predicate the return path uses. The same-arms rule comes with it, so throw e instanceof Error ? e : e is refused for the reason return x ? A : A is, and the asymmetry closes without a new laundering surface. The shared check below narrows to return statements, since the throw case is handled above. wrap-body-in-same-arms-throw-ternary is the tree-scale guard, the A/B partner of wrap-body-in-rethrow with the ternary as the only difference. Weakening the arms test takes the tree from 19 to 44 and raises 224 routes, and the entry fails; with the rule in place it reaches 391 files and 924 sites, raises none. No published figure moves. The whole JSON report over apps/webapp/app/routes is byte-identical before and after, global 19, measured 412, per-entry diffs 0, because no route in the tree writes a thrown ternary.
1 parent f6d12a1 commit 0fdf03c

3 files changed

Lines changed: 81 additions & 5 deletions

File tree

internal-packages/observability-map/src/mutations.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,18 @@ export const MUTATIONS: Mutation[] = [
502502
"try {",
503503
"} catch (obsMapMutationError) { throw obsMapMutationError; }"
504504
),
505+
// The A/B partner of the entry above: the only difference is the ternary. `error-classification`
506+
// asks whether ANY reachable catch decides, so one deciding clause bought at zero cost would take
507+
// every route in the tree to a pass. That is what the same-arms rule in `selectsAnErrorPath`
508+
// refuses, and this is the tree-scale proof of it on the throw path, which was untested while
509+
// the throw path could not credit a ternary at all.
510+
wrapEveryBody(
511+
"wrap-body-in-same-arms-throw-ternary",
512+
"wrap every route body in try { ... } catch (e) { throw e instanceof Error ? e : e }",
513+
"try {",
514+
"} catch (obsMapMutationError) { throw obsMapMutationError instanceof Error " +
515+
"? obsMapMutationError : obsMapMutationError; }"
516+
),
505517
wrapEveryBody(
506518
"wrap-body-in-trace",
507519
'wrap every route body in trace("x", async () => { ... })',
@@ -869,6 +881,7 @@ export const ADDITIVE_IDS = [
869881
"dead-throw-after-switch",
870882
"dead-throw-after-try-finally",
871883
"wrap-body-in-rethrow",
884+
"wrap-body-in-same-arms-throw-ternary",
872885
"empty-instanceof-if",
873886
"registered-throw",
874887
"fake-require-guard",

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1974,6 +1974,58 @@ describe("a ternary on the error has to send its arms somewhere different", () =
19741974
expect(ep!.catches[0]!.branches).toBe(true);
19751975
});
19761976

1977+
// S6. The same three cases on the throw path, which read none of them. The throw arm of the
1978+
// shared branch check was unreachable, because the walk sets `rethrows` and cuts the path first,
1979+
// so a thrown ternary was never offered to `selectsAnErrorPath` and every one of these clauses
1980+
// read as inert. Reading it in the throw arm, before the path is cut, uses the same predicate,
1981+
// so the arm test arrives with it. `wrap-body-in-same-arms-throw-ternary` is the tree-scale
1982+
// version of the refusal.
1983+
const throwing = (value: string) => `
1984+
export async function loader() {
1985+
try {
1986+
return await prisma.thing.findMany();
1987+
} catch (error) {
1988+
throw ${value};
1989+
}
1990+
}
1991+
`;
1992+
1993+
it("credits a thrown ternary whose arms go somewhere different", () => {
1994+
const ep = scanFile("x.ts", throwing("error instanceof Response ? error : new Error('x')"));
1995+
expect(ep!.catches[0]!.branches).toBe(true);
1996+
});
1997+
1998+
it("does not credit a thrown ternary whose arms are identical", () => {
1999+
const ep = scanFile("x.ts", throwing("error instanceof Error ? error : error"));
2000+
expect(ep!.catches[0]!.branches).toBe(false);
2001+
});
2002+
2003+
it("does not credit a thrown ternary whose arms differ only in parentheses", () => {
2004+
const ep = scanFile("x.ts", throwing("error instanceof Error ? ((error)) : (error)"));
2005+
expect(ep!.catches[0]!.branches).toBe(false);
2006+
});
2007+
2008+
it("does not credit a thrown ternary that never reads the caught binding", () => {
2009+
const ep = scanFile("x.ts", throwing("other instanceof Error ? error : new Error('x')"));
2010+
expect(ep!.catches[0]!.branches).toBe(false);
2011+
});
2012+
2013+
// The throw still cuts the path, so a decision written after it is dead and stays uncredited.
2014+
it("does not credit a thrown ternary written after the clause already threw", () => {
2015+
const ep = scanFile(
2016+
"x.ts",
2017+
`export async function loader() {
2018+
try {
2019+
return await prisma.thing.findMany();
2020+
} catch (error) {
2021+
throw error;
2022+
throw error instanceof Response ? error : new Error('x');
2023+
}
2024+
}`
2025+
);
2026+
expect(ep!.catches[0]!.branches).toBe(false);
2027+
});
2028+
19772029
// The same comparison on the `if` path, which had the exit test but not the arm test.
19782030
it("does not credit an if/else whose two arms are identical", () => {
19792031
const ep = scanFile(

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

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,9 @@ function normalizedText(node: ts.Node): string {
455455
* The two arms also have to differ, which is the same requirement `selectsADistinctPath` makes of
456456
* an `if`. `return e instanceof Error ? (X) : (X)` is a test whose outcome is the same either way,
457457
* and it was worth 50 points a route; `same-arms-ternary` in the mutation corpus is the tree-scale
458-
* version, and `scan.test.ts` has the unit case. Parentheses and whitespace are stripped
458+
* version, and `scan.test.ts` has the unit case. The throw path is held to the same rule by
459+
* `wrap-body-in-same-arms-throw-ternary`, which would take every route in the tree to a pass if it
460+
* were not. Parentheses and whitespace are stripped
459461
* before the comparison, so the shape has to differ in something a reader would call a difference.
460462
* The residual both branch tests share is stated once, on `selectsADistinctPath`.
461463
*/
@@ -628,6 +630,18 @@ function catchClauseEvidence(clause: ts.CatchClause): {
628630
for (const statement of reachableStatements(statements)) {
629631
if (ts.isThrowStatement(statement)) {
630632
rethrows = true;
633+
// Read the branch check here, before the path is cut. A thrown ternary picks WHICH error
634+
// leaves, which is a classification, and reading it only at the shared check below meant
635+
// the throw arm of that condition was unreachable: this arm always continued first. So
636+
// `throw e instanceof Response ? e : new ServerError(e)` read as inert while the same
637+
// clause written with `return` passed. `selectsAnErrorPath` is the same predicate either
638+
// way, so the same-arms rule applies and `throw e instanceof Error ? e : e;` is refused.
639+
if (bindingName !== null && !shadowed && !exited && statement.expression !== undefined) {
640+
const thrown = unwrap(statement.expression);
641+
if (ts.isConditionalExpression(thrown) && selectsAnErrorPath(thrown, bindingName)) {
642+
branches = true;
643+
}
644+
}
631645
exited = true;
632646
continue;
633647
}
@@ -656,10 +670,7 @@ function catchClauseEvidence(clause: ts.CatchClause): {
656670
selectsADistinctPath(statement)
657671
) {
658672
branches = true;
659-
} else if (
660-
(ts.isReturnStatement(statement) || ts.isThrowStatement(statement)) &&
661-
statement.expression !== undefined
662-
) {
673+
} else if (ts.isReturnStatement(statement) && statement.expression !== undefined) {
663674
const value = unwrap(statement.expression);
664675
if (ts.isConditionalExpression(value) && selectsAnErrorPath(value, bindingName)) {
665676
branches = true;

0 commit comments

Comments
 (0)