Skip to content

Commit

Permalink
fix: wing console cannot be started (hangs) (#5924)
Browse files Browse the repository at this point in the history
Wing Console used `tryGetResource` and expected it to return `undefined` if the node was not found, but #5821 did not respect this contract.

Fixed and added a test.

Fixes #5923 Fixes #5922

## Checklist

- [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [x] Tests added (always)
- [x] Docs updated (only required for features)
- [x] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
  • Loading branch information
eladb authored Mar 12, 2024
1 parent 67e70e4 commit f4cd778
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 18 deletions.
10 changes: 5 additions & 5 deletions libs/wingsdk/src/simulator/graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,18 @@ export class Graph<T extends Definition> {
return Object.values(this.byPath);
}

public find(path: string): Node<T> {
public tryFind(path: string): Node<T> | undefined {
const node = this.byPath[path];
if (!node) {
throw new Error(`node not found: ${path}`);
return undefined;
}

return node;
}

private recordDependency(consumer: string, producer: string) {
this.find(consumer).dependencies.add(producer);
this.find(producer).dependents.add(consumer);
this.tryFind(consumer)?.dependencies.add(producer);
this.tryFind(producer)?.dependents.add(consumer);

// check for cyclic dependencies
this.detectCycles(consumer);
Expand All @@ -91,7 +91,7 @@ export class Graph<T extends Definition> {
visited.add(path);
stack.add(path);

for (const dep of this.find(path).dependencies) {
for (const dep of this.tryFind(path)?.dependencies ?? []) {
visit(dep);
}

Expand Down
14 changes: 9 additions & 5 deletions libs/wingsdk/src/simulator/simulator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ export class Simulator {

private async startResources() {
const retries: Record<string, number> = {};
const queue = this._model.graph.nodes.map(n => n.path);
const queue = this._model.graph.nodes.map((n) => n.path);
while (queue.length > 0) {
const top = queue.shift()!;
try {
Expand Down Expand Up @@ -367,7 +367,7 @@ export class Simulator {
}

// first, stop all dependent resources
for (const consumer of this._model.graph.find(path)?.dependents ?? []) {
for (const consumer of this._model.graph.tryFind(path)?.dependents ?? []) {
await this.stopResource(consumer);
}

Expand Down Expand Up @@ -471,7 +471,11 @@ export class Simulator {
path = `root${path}`;
}

const def = this._model.graph.find(path).def;
const def = this._model.graph.tryFind(path)?.def;
if (!def) {
return undefined;
}

const state = this.state[path];

return {
Expand Down Expand Up @@ -677,7 +681,7 @@ export class Simulator {
}

// first lets make sure all my dependencies have been started (depth-first)
for (const d of this._model.graph.find(path).dependencies) {
for (const d of this._model.graph.tryFind(path)?.dependencies ?? []) {
await this.startResource(d);
}

Expand Down Expand Up @@ -806,7 +810,7 @@ export class Simulator {
*/
private resolveTokens(obj: any): any {
return resolveTokens(obj, (token) => {
const target = this._model.graph.find(token.path);
const target = this._model.graph.tryFind(token.path);
if (!target) {
throw new Error(
`Could not resolve token "${token}" because the resource at path "${token.path}" does not exist.`
Expand Down
21 changes: 13 additions & 8 deletions libs/wingsdk/test/simulator/graph.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ test("two disconnected nodes", () => {

expect(graph.nodes.length).toBe(2);

const a = graph.find("a");
const a = graph.tryFind("a")!;
expect(a.def).toStrictEqual({ path: "a" });
expect(Array.from(a.dependencies)).toStrictEqual([]);
expect(Array.from(a.dependents)).toStrictEqual([]);

const b = graph.find("b");
const b = graph.tryFind("b")!;
expect(b.def).toStrictEqual({ path: "b" });
expect(Array.from(b.dependencies)).toStrictEqual([]);
expect(Array.from(b.dependents)).toStrictEqual([]);
Expand All @@ -25,11 +25,11 @@ test("two disconnected nodes", () => {
test("explicit deps", () => {
const graph = new Graph([{ path: "a", deps: ["b"] }, { path: "b" }]);

const a = graph.find("a");
const a = graph.tryFind("a")!;
expect(a.dependencies.size).toBe(1);
expect(Array.from(a.dependencies)).toStrictEqual(["b"]);

const b = graph.find("b");
const b = graph.tryFind("b")!;
expect(b.dependents.size).toBe(1);
expect(Array.from(b.dependents)).toStrictEqual(["a"]);
});
Expand All @@ -48,23 +48,28 @@ test("implicit deps", () => {
{ path: "d", props: { a: "${wsim#a#attrs.aaa}" }, deps: ["b"] },
]);

const a = graph.find("a");
const a = graph.tryFind("a")!;
expect(Array.from(a.dependencies)).toStrictEqual(["b", "c/d/e"]);
expect(Array.from(a.dependents)).toStrictEqual(["d"]);

const b = graph.find("b");
const b = graph.tryFind("b")!;
expect(Array.from(b.dependencies)).toStrictEqual(["c/d/e"]);
expect(Array.from(b.dependents)).toStrictEqual(["a", "d"]);

const c = graph.find("c/d/e");
const c = graph.tryFind("c/d/e")!;
expect(Array.from(c.dependencies)).toStrictEqual([]);
expect(Array.from(c.dependents)).toStrictEqual(["a", "b"]);

const d = graph.find("d");
const d = graph.tryFind("d")!;
expect(Array.from(d.dependencies)).toStrictEqual(["b", "a"]);
expect(Array.from(d.dependents)).toStrictEqual([]);
});

test("tryFind returns undefined if node does not exist", () => {
const graph = new Graph([]);
expect(graph.tryFind("a")).toBe(undefined);
});

test("fails on a direct cyclic dependency", () => {
expect(() => {
new Graph([
Expand Down
7 changes: 7 additions & 0 deletions libs/wingsdk/test/simulator/simulator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,13 @@ describe("in-place updates", () => {
});
});

test("tryGetResource returns undefined if the resource not found", async () => {
const app = new SimApp();
const sim = await app.startSimulator();
expect(sim.tryGetResource("bang")).toBeUndefined();
expect(sim.tryGetResourceConfig("bing")).toBeUndefined();
});

function makeTest(
scope: Construct,
id: string,
Expand Down

0 comments on commit f4cd778

Please sign in to comment.