repos / starfx

commit

912155b

parent

25ec8ea

author

Eric Bower

date

2023-07-30 20:24:51 +0000 UTC

fix: bubble errors up properly

 1@@ -6,7 +6,7 @@ import { compose } from "./compose.ts";
 2 const tests = describe("compose()");
 3 
 4 it(tests, "should compose middleware", async () => {
 5-  const mdw = compose<{ one: string; three: string; result: Result<any[]> }>([
 6+  const mdw = compose<{ one: string; three: string; result: Result<void> }>([
 7     function* (ctx, next) {
 8       ctx.one = "two";
 9       yield* next();
10@@ -17,7 +17,7 @@ it(tests, "should compose middleware", async () => {
11     },
12   ]);
13   const actual = await run(function* () {
14-    const ctx = { one: "", three: "", result: Ok([]) };
15+    const ctx = { one: "", three: "", result: Ok(void 0) };
16     yield* mdw(ctx);
17     return ctx;
18   });
19@@ -26,13 +26,13 @@ it(tests, "should compose middleware", async () => {
20     // we should see the mutation
21     one: "two",
22     three: "four",
23-    result: Ok([undefined, undefined]),
24+    result: Ok(void 0),
25   };
26   expect(actual).toEqual(expected);
27 });
28 
29 it(tests, "order of execution", async () => {
30-  const mdw = compose<{ actual: string; result: Result<any[]> }>([
31+  const mdw = compose<{ actual: string; result: Result<void> }>([
32     function* (ctx, next) {
33       ctx.actual += "a";
34       yield* next();
35@@ -55,55 +55,31 @@ it(tests, "order of execution", async () => {
36   ]);
37 
38   const actual = await run(function* () {
39-    const ctx = { actual: "", result: Ok([]) };
40+    const ctx = { actual: "", result: Ok(void 0) };
41     yield* mdw(ctx);
42     return ctx;
43   });
44   const expected = {
45     actual: "abcdefg",
46-    result: Ok([undefined, undefined, undefined]),
47+    result: Ok(void 0),
48   };
49   expect(actual).toEqual(expected);
50 });
51 
52-it(tests, "result of each mdw is aggregated to `ctx.result`", async () => {
53-  const mdw = compose<{ result: Result<any[]> }>([
54-    function* (_, next) {
55-      yield* next();
56-      return "two";
57-    },
58-    function* (_, next) {
59-      yield* next();
60-      return "one";
61-    },
62-  ]);
63-  const actual = await run(function* () {
64-    const ctx = { result: Ok([]) };
65-    yield* mdw(ctx);
66-    return ctx;
67-  });
68-
69-  const expected = {
70-    result: Ok(["one", "two"]),
71-  };
72-
73-  expect(actual).toEqual(expected);
74-});
75-
76 it(tests, "when error is discovered return in `ctx.result`", async () => {
77   const err = new Error("boom");
78-  const mdw = compose<{ result: Result<any[]> }>([
79+  const mdw = compose<{ result: Result<void> }>([
80     function* (_, next) {
81       yield* next();
82-      throw err;
83+      asserts.fail();
84     },
85     function* (_, next) {
86       yield* next();
87-      asserts.fail();
88+      throw err;
89     },
90   ]);
91   const actual = await run(function* () {
92-    const ctx = { result: Ok([]) };
93+    const ctx = { result: Ok(undefined) };
94     yield* mdw(ctx);
95     return ctx;
96   });

 1@@ -1,13 +1,11 @@
 2 import { call } from "./fx/mod.ts";
 3 import type { Next } from "./query/mod.ts";
 4-import { Err, Instruction, Operation, Result } from "./deps.ts";
 5-import { resultAll } from "./result.ts";
 6+import { Instruction, Operation, Result } from "./deps.ts";
 7 
 8-// deno-lint-ignore no-explicit-any
 9-export interface BaseCtx<T extends any[] = any[]> {
10+export interface BaseCtx {
11   // deno-lint-ignore no-explicit-any
12   [key: string]: any;
13-  result: Result<T>;
14+  result: Result<void>;
15 }
16 
17 export type BaseMiddleware<Ctx extends BaseCtx = BaseCtx, T = unknown> = (
18@@ -29,15 +27,12 @@ export function compose<Ctx extends BaseCtx = BaseCtx, T = unknown>(
19   }
20 
21   return function* composeFn(context: Ctx, mdw?: BaseMiddleware<Ctx, T>) {
22-    // deno-lint-ignore no-explicit-any
23-    const results: Result<any>[] = [];
24     // last called middleware #
25     let index = -1;
26 
27     function* dispatch(i: number): Generator<Instruction, void, void> {
28       if (i <= index) {
29-        results.push(Err(new Error("next() called multiple times")));
30-        return;
31+        throw new Error("next() called multiple times");
32       }
33       index = i;
34       let fn: BaseMiddleware<Ctx, T> | undefined = middleware[i];
35@@ -48,22 +43,18 @@ export function compose<Ctx extends BaseCtx = BaseCtx, T = unknown>(
36         return;
37       }
38       const nxt = dispatch.bind(null, i + 1);
39-      // wrap mdw in a safe call
40-      const result = yield* call(() =>
41-        (fn as BaseMiddleware<Ctx, T>)(context, nxt)
42-      );
43-
44-      // exit early if an error is discovered
45-      if (!result.ok) {
46+      const result = yield* call(function* () {
47+        if (!fn) return;
48+        return yield* fn(context, nxt);
49+      });
50+      if (!result.ok && context.result.ok) {
51         context.result = result;
52-        return;
53       }
54-
55-      results.push(result);
56-      // aggregate results on each pass of the mdw
57-      context.result = resultAll(results);
58     }
59 
60-    yield* dispatch(0);
61+    const result = yield* call(() => dispatch(0));
62+    if (context.result.ok) {
63+      context.result = result;
64+    }
65   };
66 }

 1@@ -1,6 +1,6 @@
 2 import type { OpFn } from "../types.ts";
 3-import type { Operation, Result, Task } from "../deps.ts";
 4-import { action, Err, expect, Ok, spawn } from "../deps.ts";
 5+import type { Operation, Result } from "../deps.ts";
 6+import { action, Err, expect, Ok } from "../deps.ts";
 7 
 8 export const isFunc = (f: unknown) => typeof f === "function";
 9 export const isPromise = (p: unknown) =>
10@@ -38,10 +38,3 @@ export function* call<T>(opFn: OpFn<T>): Operation<Result<T>> {
11     return Err(error);
12   }
13 }
14-
15-export function* go<T>(op: OpFn<T>): Operation<Task<Result<T>>> {
16-  return yield* spawn(function* () {
17-    const result = yield* call(op);
18-    return result;
19-  });
20-}

 1@@ -430,3 +430,33 @@ it(
 2     expect(acc).toEqual(["wow"]);
 3   },
 4 );
 5+
 6+it(tests, "should bubble up error", async () => {
 7+  let error: any = null;
 8+  const api = createApi();
 9+  api.use(function* (ctx, next) {
10+    yield* next();
11+    if (!ctx.result.ok) {
12+      error = ctx.result.error;
13+    }
14+  });
15+  api.use(queryCtx);
16+  api.use(storeMdw());
17+  api.use(api.routes());
18+
19+  const fetchUser = api.get(
20+    "/users/8",
21+    { supervisor: takeEvery },
22+    function* (ctx, _) {
23+      (ctx.loader as any).meta = { key: ctx.payload.thisKeyDoesNotExist };
24+      throw new Error("GENERATING AN ERROR");
25+    },
26+  );
27+
28+  const store = await configureStore({ initialState: { users: {} } });
29+  store.run(api.bootup);
30+  store.dispatch(fetchUser());
31+  expect(error.message).toBe(
32+    "Cannot read properties of undefined (reading 'thisKeyDoesNotExist')",
33+  );
34+});

 1@@ -70,13 +70,13 @@ export function createApi<Ctx extends ApiCtx = ApiCtx>(
 2     cache: () => {
 3       return function* onCache(ctx: Ctx, next: Next) {
 4         ctx.cache = true;
 5-        yield next();
 6+        yield* next();
 7       };
 8     },
 9     request: (req: ApiRequest) => {
10       return function* onRequest(ctx: Ctx, next: Next) {
11         ctx.request = ctx.req(req);
12-        yield next();
13+        yield* next();
14       };
15     },
16     uri,

  1@@ -33,6 +33,7 @@ it(
  2     api.use(api.routes());
  3     api.use(fetcher({ baseUrl }));
  4 
  5+    const actual: any[] = [];
  6     const fetchUsers = api.get(
  7       "/users",
  8       { supervisor: takeEvery },
  9@@ -40,15 +41,8 @@ it(
 10         ctx.cache = true;
 11         yield* next();
 12 
 13-        expect(ctx.request).toEqual({
 14-          url: `${baseUrl}/users`,
 15-          method: "GET",
 16-          headers: {
 17-            "Content-Type": "application/json",
 18-          },
 19-        });
 20-
 21-        expect(ctx.json).toEqual({ ok: true, data: mockUser });
 22+        actual.push(ctx.request);
 23+        actual.push(ctx.json);
 24       },
 25     );
 26 
 27@@ -64,6 +58,14 @@ it(
 28 
 29     const state = store.getState();
 30     expect(state["@@starfx/data"][action.payload.key]).toEqual(mockUser);
 31+
 32+    expect(actual).toEqual([{
 33+      url: `${baseUrl}/users`,
 34+      method: "GET",
 35+      headers: {
 36+        "Content-Type": "application/json",
 37+      },
 38+    }, { ok: true, data: mockUser }]);
 39   },
 40 );
 41 
 42@@ -81,14 +83,15 @@ it(
 43     api.use(api.routes());
 44     api.use(fetcher({ baseUrl }));
 45 
 46+    let actual = null;
 47     const fetchUsers = api.get(
 48       "/users",
 49       { supervisor: takeEvery },
 50       function* (ctx, next) {
 51         ctx.cache = true;
 52         ctx.bodyType = "text";
 53-        yield next();
 54-        expect(ctx.json).toEqual({ ok: true, data: "this is some text" });
 55+        yield* next();
 56+        actual = ctx.json;
 57       },
 58     );
 59 
 60@@ -101,6 +104,7 @@ it(
 61     store.dispatch(action);
 62 
 63     await delay();
 64+    expect(actual).toEqual({ ok: true, data: "this is some text" });
 65   },
 66 );
 67 
 68@@ -121,6 +125,7 @@ it(tests, "fetch - error handling", async () => {
 69   });
 70   api.use(fetcher());
 71 
 72+  let actual = null;
 73   const fetchUsers = api.get(
 74     "/users",
 75     { supervisor: takeEvery },
 76@@ -128,7 +133,7 @@ it(tests, "fetch - error handling", async () => {
 77       ctx.cache = true;
 78       yield* next();
 79 
 80-      expect(ctx.json).toEqual({ ok: false, data: errMsg });
 81+      actual = ctx.json;
 82     },
 83   );
 84 
 85@@ -144,11 +149,12 @@ it(tests, "fetch - error handling", async () => {
 86 
 87   const state = store.getState();
 88   expect(state["@@starfx/data"][action.payload.key]).toEqual(errMsg);
 89+  expect(actual).toEqual({ ok: false, data: errMsg });
 90 });
 91 
 92 it(tests, "fetch - status 204", async () => {
 93   mock(`GET@/users`, () => {
 94-    return new Response("", { status: 204 });
 95+    return new Response(null, { status: 204 });
 96   });
 97 
 98   const api = createApi();
 99@@ -162,14 +168,14 @@ it(tests, "fetch - status 204", async () => {
100   });
101   api.use(fetcher());
102 
103+  let actual = null;
104   const fetchUsers = api.get(
105     "/users",
106     { supervisor: takeEvery },
107     function* (ctx, next) {
108       ctx.cache = true;
109       yield* next();
110-
111-      expect(ctx.json).toEqual({ ok: true, data: {} });
112+      actual = ctx.json;
113     },
114   );
115 
116@@ -185,6 +191,7 @@ it(tests, "fetch - status 204", async () => {
117 
118   const state = store.getState();
119   expect(state["@@starfx/data"][action.payload.key]).toEqual({});
120+  expect(actual).toEqual({ ok: true, data: {} });
121 });
122 
123 it(tests, "fetch - malformed json", async () => {
124@@ -203,6 +210,7 @@ it(tests, "fetch - malformed json", async () => {
125   });
126   api.use(fetcher());
127 
128+  let actual = null;
129   const fetchUsers = api.get(
130     "/users",
131     { supervisor: takeEvery },
132@@ -210,13 +218,7 @@ it(tests, "fetch - malformed json", async () => {
133       ctx.cache = true;
134       yield* next();
135 
136-      expect(ctx.json).toEqual({
137-        ok: false,
138-        data: {
139-          message:
140-            "invalid json response body at https://saga-query.com/users reason: Unexpected token o in JSON at position 1",
141-        },
142-      });
143+      actual = ctx.json;
144     },
145   );
146 
147@@ -228,6 +230,13 @@ it(tests, "fetch - malformed json", async () => {
148   store.dispatch(action);
149 
150   await delay();
151+
152+  expect(actual).toEqual({
153+    ok: false,
154+    data: {
155+      message: "Unexpected token 'o', \"not json\" is not valid JSON",
156+    },
157+  });
158 });
159 
160 it(tests, "fetch - POST", async () => {
161@@ -394,16 +403,17 @@ it(
162     api.use(api.routes());
163     api.use(fetcher({ baseUrl }));
164 
165+    let actual = null;
166     const fetchUsers = api.get("/users", { supervisor: takeEvery }, [
167       function* (ctx, next) {
168         ctx.cache = true;
169         yield* next();
170 
171         if (!ctx.json.ok) {
172-          expect(true).toBe(false);
173+          return;
174         }
175 
176-        expect(ctx.json).toEqual({ ok: true, data: mockUser });
177+        actual = ctx.json;
178       },
179       fetchRetry((n) => (n > 4 ? -1 : 10)),
180     ]);
181@@ -420,10 +430,11 @@ it(
182 
183     const state = store.getState();
184     expect(state["@@starfx/data"][action.payload.key]).toEqual(mockUser);
185+    expect(actual).toEqual({ ok: true, data: mockUser });
186   },
187 );
188 
189-it.ignore(
190+it(
191   tests,
192   "fetch retry - with failure - should keep retrying and then quit",
193   async () => {
194@@ -433,6 +444,7 @@ it.ignore(
195       });
196     });
197 
198+    let actual = null;
199     const api = createApi();
200     api.use(requestMonitor());
201     api.use(storeMdw());
202@@ -443,7 +455,7 @@ it.ignore(
203       function* (ctx, next) {
204         ctx.cache = true;
205         yield* next();
206-        expect(ctx.json).toEqual({ ok: false, data: { message: "error" } });
207+        actual = ctx.json;
208       },
209       fetchRetry((n) => (n > 2 ? -1 : 10)),
210     ]);
211@@ -456,5 +468,6 @@ it.ignore(
212     store.dispatch(action);
213 
214     await delay();
215+    expect(actual).toEqual({ ok: false, data: { message: "error" } });
216   },
217 );

 1@@ -99,14 +99,14 @@ export interface SagaApi<Ctx extends PipeCtx> {
 2  * const thunk = createPipe();
 3  * thunk.use(function* (ctx, next) {
 4  *   console.log('beginning');
 5- *   yield next();
 6+ *   yield* next();
 7  *   console.log('end');
 8  * });
 9  * thunks.use(thunks.routes());
10  *
11  * const doit = thunk.create('do-something', function*(ctx, next) {
12  *   console.log('middle');
13- *   yield next();
14+ *   yield* next();
15  *   console.log('middle end');
16  * });
17  *

 1@@ -38,12 +38,12 @@ const setupTest = async () => {
 2     yield* delay(10);
 3     ctx.json = { ok: true, data: mockUser };
 4     ctx.response = new Response(jsonBlob(mockUser), { status: 200 });
 5-    yield next();
 6+    yield* next();
 7   });
 8 
 9   const fetchUser = api.get<{ id: string }>("/user/:id", function* (ctx, next) {
10     ctx.cache = true;
11-    yield next();
12+    yield* next();
13     if (!ctx.json.ok) return;
14     yield* updateStore<{ user: User }>((state) => {
15       state.user = ctx.json.data;

1@@ -12,7 +12,7 @@ export interface PipeCtx<P = any> extends Payload<P> {
2     CreateAction<PipeCtx>,
3     CreateActionWithPayload<PipeCtx<P>, P>
4   >;
5-  result: Result<unknown[]>;
6+  result: Result<void>;
7 }
8 
9 export interface LoaderCtx<P = unknown> extends PipeCtx<P> {

1@@ -114,7 +114,7 @@ export function createStore<S extends AnyState>({
2     const ctx = {
3       updater,
4       patches: [],
5-      result: Ok([]),
6+      result: Ok(undefined),
7     };
8     yield* mdw(ctx);
9     // TODO: dev mode only?