Skip to content

Commit c531bb3

Browse files
authored
feat(runner): throw errors instead of returning empty document on link cycles (#1925)
Changed link resolution behavior to throw errors when detecting cycles or hitting iteration limits, rather than silently returning an empty document with a `did:null:null` space. Changes: - Modified `resolveLink()` to throw when cycle detected or iteration limit reached - Removed `createEmptyResolvedFullLink()` helper that created the empty document - Updated all related tests to expect thrown errors instead of undefined values - Changed logger severity from warn to error for these conditions - Enhanced cycle detection error message to include the seen path for debugging This makes link resolution errors more explicit and easier to debug, rather than silently failing with an empty document that might be missed.
1 parent 20261cb commit c531bb3

File tree

2 files changed

+21
-38
lines changed

2 files changed

+21
-38
lines changed

packages/runner/src/link-resolution.ts

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,17 @@ export function resolveLink(
7070

7171
while (true) {
7272
if (iteration++ > MAX_PATH_RESOLUTION_LENGTH) {
73-
logger.warn(`Link resolution iteration limit reached`);
74-
return createEmptyResolvedFullLink(link); // = return link to empty document
73+
logger.error(`Link resolution iteration limit reached`);
74+
throw new Error(`Link resolution iteration limit reached`);
7575
}
7676

7777
// Detect cycles.
7878
const key = JSON.stringify([link.space, link.id, link.path]);
7979
if (seen.has(key)) {
80-
logger.warn(`Link cycle detected ${key}`);
81-
return createEmptyResolvedFullLink(link); // = return link to empty document
80+
logger.error(`Link cycle detected ${key} [${JSON.stringify([...seen])}]`);
81+
throw new Error(
82+
`Link cycle detected at ${key} [${JSON.stringify([...seen])}]`,
83+
);
8284
}
8385
seen.add(key);
8486

@@ -258,15 +260,3 @@ export function readMaybeLink(
258260
return undefined;
259261
}
260262
}
261-
262-
function createEmptyResolvedFullLink(
263-
link: NormalizedFullLink,
264-
): ResolvedFullLink {
265-
return {
266-
...link,
267-
id: "data:application/json,{}",
268-
path: [],
269-
type: "application/json",
270-
space: "did:null:null",
271-
} satisfies NormalizedFullLink as unknown as ResolvedFullLink;
272-
}

packages/runner/test/link-resolution.test.ts

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1005,8 +1005,7 @@ describe("link-resolution", () => {
10051005

10061006
// When we resolve a circular reference, it should return undefined
10071007
// (the empty document resolves to undefined)
1008-
const value = cellA.get();
1009-
expect(value).toBeUndefined();
1008+
expect(() => cellA.get()).toThrow();
10101009

10111010
// Case 2: Mutual references A -> B -> A
10121011
const cellB = runtime.getCell<any>(
@@ -1024,8 +1023,8 @@ describe("link-resolution", () => {
10241023
cellB.setRaw(linkToA);
10251024

10261025
// Both should resolve to undefined
1027-
expect(cellA.get()).toBeUndefined();
1028-
expect(cellB.get()).toBeUndefined();
1026+
expect(() => cellA.get()).toThrow();
1027+
expect(() => cellB.get()).toThrow();
10291028
});
10301029

10311030
it("detects cycle when resolving link to its own subpath", async () => {
@@ -1056,15 +1055,10 @@ describe("link-resolution", () => {
10561055
});
10571056

10581057
try {
1059-
const resolved = await Promise.race([
1060-
resolutionPromise,
1061-
timeoutPromise,
1062-
]);
1063-
1064-
// This creates: A -> A/foo -> A/foo/foo -> A/foo/foo/foo -> ...
1065-
// The iteration limit should catch this and return the empty document
1066-
expect(resolved.id).toBe("data:application/json,{}");
1067-
expect(resolved.space).toBe("did:null:null");
1058+
await expect(Promise.race([resolutionPromise, timeoutPromise])).rejects
1059+
.toThrow(
1060+
"Link resolution iteration limit reached",
1061+
);
10681062
} finally {
10691063
// Clean up the timeout if it was set
10701064
if (timeoutId !== undefined) {
@@ -1095,8 +1089,7 @@ describe("link-resolution", () => {
10951089
cellA.setRaw({ x: linkToBy });
10961090
cellB.setRaw({ y: linkToAx });
10971091

1098-
const value = cellA.key("x").get();
1099-
expect(value).toBeUndefined();
1092+
expect(() => cellA.key("x").get()).toThrow();
11001093
});
11011094

11021095
it("shows data URI when resolving cyclic links", () => {
@@ -1114,13 +1107,13 @@ describe("link-resolution", () => {
11141107

11151108
// When we resolve this link at a low level, it should return the empty document
11161109
// with a data: URI indicating the cycle was detected
1117-
const resolved = resolveLink(
1118-
tx,
1119-
cellA.getAsNormalizedFullLink(),
1120-
"value",
1121-
);
1122-
expect(resolved.id).toBe("data:application/json,{}");
1123-
expect(resolved.space).toBe("did:null:null");
1110+
expect(() =>
1111+
resolveLink(
1112+
tx,
1113+
cellA.getAsNormalizedFullLink(),
1114+
"value",
1115+
)
1116+
).toThrow();
11241117
});
11251118

11261119
it("allows non-cyclic references to the same cell", () => {

0 commit comments

Comments
 (0)