From 8e2f8b8d9fbdab20368dcd7f7eddd0816fd9daf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Guyon?= Date: Thu, 24 May 2018 18:48:48 +0200 Subject: [PATCH 1/4] Add possibility to reject the future --- src/Future.re | 76 ++++++++++++----- tests/TestFuture.re | 193 +++++++++++++++++++++++++++++++------------- 2 files changed, 193 insertions(+), 76 deletions(-) diff --git a/src/Future.re b/src/Future.re index 31f55c8..e803e17 100644 --- a/src/Future.re +++ b/src/Future.re @@ -1,46 +1,80 @@ -type getFn('a) = ('a => unit) => unit; +type getFn('a) = ('a => unit, exn => unit) => unit; type t('a) = Future(getFn('a)); let make = (resolver) => { - let callbacks = ref([]); + open Belt; + + let successCallbacks = ref([]); + let failureCallbacks = ref([]); let data = ref(None); - resolver(result => switch(data^) { + resolver( + result => switch(data^) { + | None => + data := Some(Result.Ok(result)); + successCallbacks^ |. List.reverse |. List.forEach(cb => cb(result)); + /* Clean up memory usage */ + successCallbacks := []; + failureCallbacks := [] + | Some(_) => + () /* Do nothing; theoretically not possible */ + }, + error => switch (data^) { + | None => + data := Some(Result.Error(error)); + failureCallbacks^ |. List.reverse |. List.forEach(cb => cb(error)); + + successCallbacks := []; + failureCallbacks := [] + | Some(_) => + () + } + ); + + Future((resolve, reject) => switch(data^) { + | Some(Ok(result)) => resolve(result) + | Some(Error(error)) => reject(error) | None => - data := Some(result); - callbacks^ |. Belt.List.reverse |. Belt.List.forEach(cb => cb(result)); - /* Clean up memory usage */ - callbacks := [] - | Some(_) => - () /* Do nothing; theoretically not possible */ - }); - - Future(resolve => switch(data^) { - | Some(result) => resolve(result) - | None => callbacks := [resolve, ...callbacks^] + successCallbacks := [resolve, ...successCallbacks^]; + failureCallbacks := [reject, ...failureCallbacks^]; }) }; -let value = (x) => make(resolve => resolve(x)); +let value = (x) => make((resolve, _reject) => resolve(x)); + +let error = (e) => make((_resolve, reject) => reject(e)); let map = (Future(get), f) => make(resolve => { get(result => resolve(f(result))) }); -let flatMap = (Future(get), f) => make(resolve => { - get(result => { - let Future(get2) = f(result); - get2(resolve) - }) +let flatMap = (Future(get), f) => make((resolve, reject) => { + get( + result => { + let Future(get2) = f(result); + get2(resolve, reject) + }, + reject + ) }); let tap = (Future(get) as future, f) => { - get(f); + get(f, _error => ()); future }; +let catch = (Future(get), f) => make((resolve, reject) => { + get( + resolve, + error => { + let Future(get2) = f(error); + get2(resolve, reject) + } + ) +}); + let get = (Future(getFn), f) => getFn(f); /* * diff --git a/tests/TestFuture.re b/tests/TestFuture.re index 77ff591..bfc4764 100644 --- a/tests/TestFuture.re +++ b/tests/TestFuture.re @@ -4,28 +4,69 @@ exception TestError(string); type timeoutId; [@bs.val] [@bs.val] external setTimeout : ([@bs.uncurry] (unit => unit), int) => timeoutId = ""; +exception Err(string); + describe("Future", () => { - let delay = (ms, f) => Future.make(resolve => + let delay = (ms, f) => Future.make((resolve, _reject) => setTimeout(() => f() |> resolve, ms) |> ignore ); + let delayError = (ms, f) => Future.make((_resolve, reject) => + setTimeout(() => f() |> reject, ms) |> ignore + ); + test("sync chaining", () => { Future.value("one") |. Future.map(s => s ++ "!") - |. Future.map(s => { - s |. equals("one!"); - }); + |. Future.get( + s => { + s |. equals("one!"); + }, + _ => () + ) + }); + + test("sync error chaining", () => { + Future.error(Err("one")) + |. Future.catch(e => switch (e) { + | Err(s) => Err(s ++ "!") |. Future.error + | _ => assert false + }) + |. Future.get( + _ => (), + err => { + err |. deepEquals(Err("one!")); + } + ) }); testAsync("async chaining", done_ => { delay(25, () => 20) |. Future.map(s => string_of_int(s)) |. Future.map(s => s ++ "!") - |. Future.get(s => { - s |. equals("20!"); - done_(); - }); + |. Future.get( + s => { + s |. equals("20!"); + done_(); + }, + _ => () + ); + }); + + testAsync("async error chaining", done_ => { + delayError(25, () => Err("20")) + |. Future.catch(err => switch (err) { + | Err(s) => Err(s ++ "!") |. Future.error + | _ => assert false + }) + |. Future.get( + _ => (), + error => { + error |. deepEquals(Err("20!")); + done_(); + } + ); }); test("tap", () => { @@ -34,36 +75,48 @@ describe("Future", () => { Future.value(99) |. Future.tap(n => v := n+1) |. Future.map(n => n - 9) - |. Future.get(n => { - n |. equals(90); - v^ |. equals(100); - }); + |. Future.get( + n => { + n |. equals(90); + v^ |. equals(100); + }, + _ => () + ); }); test("flatMap", () => { Future.value(59) |. Future.flatMap(n => Future.value(n + 1)) - |. Future.get(n => { - n |. equals(60); - }); + |. Future.get( + n => { + n |. equals(60); + }, + _ => () + ); }); test("multiple gets", () => { let count = ref(0); - let future = Future.make(resolve => { + let future = Future.make((resolve, _reject) => { count := count^ + 1; resolve(count^); }); count^ |. equals(1); - future |. Future.get(c => { - c |. equals(1); - }); + future |. Future.get( + c => { + c |. equals(1); + }, + _ => () + ); count^ |. equals(1); - future |. Future.get(c => { - c |. equals(1); - }); + future |. Future.get( + c => { + c |. equals(1); + }, + _ => () + ); count^ |. equals(1); }); @@ -76,17 +129,23 @@ describe("Future", () => { count^ |. equals(~m="Callback is async", 0); - future |. Future.get(_ => { - count^ |. equals(~m="Runs after previous future", 1); - }); + future |. Future.get( + _ => { + count^ |. equals(~m="Runs after previous future", 1); + }, + _ => () + ); count^ |. equals(~m="Callback is async (2)", 0); - future |. Future.get(_ => { - count^ |. equals(~m="Previous future only runs once", 1); - }); + future |. Future.get( + _ => { + count^ |. equals(~m="Previous future only runs once", 1); + }, + _ => () + ); count^ |. equals(0, ~m="Callback is async (3)"); - future |. Future.get(_ => done_()); + future |. Future.get(_ => done_(), _ => ()); }); }); @@ -98,34 +157,46 @@ describe("Future Belt.Result", () => { Belt.Result.Ok("two") |. Future.value |. Future.mapOk(s => s ++ "!") - |. Future.get(r => { - Belt.Result.getExn(r) |. equals("two!"); - }); + |. Future.get( + r => { + Belt.Result.getExn(r) |. equals("two!"); + }, + _ => () + ); Belt.Result.Error("err2") |. Future.value |. Future.mapOk(s => s ++ "!") - |. Future.get(r => switch (r) { - | Ok(_) => raise(TestError("shouldn't be possible")) - | Error(e) => e |. equals("err2"); - }); + |. Future.get( + r => switch (r) { + | Ok(_) => raise(TestError("shouldn't be possible")) + | Error(e) => e |. equals("err2"); + }, + _ => () + ); }); test("mapError", () => { Belt.Result.Ok("three") |. Future.value |. Future.mapError(s => s ++ "!") - |. Future.get(r => { - Belt.Result.getExn(r) |. equals("three"); - }); + |. Future.get( + r => { + Belt.Result.getExn(r) |. equals("three"); + }, + _ => () + ); Belt.Result.Error("err3") |. Future.value |. Future.mapError(s => s ++ "!") - |. Future.get(r => switch (r) { - | Ok(_) => raise(TestError("shouldn't be possible")) - | Error(e) => e |. equals("err3!"); - }); + |. Future.get( + r => switch (r) { + | Ok(_) => raise(TestError("shouldn't be possible")) + | Error(e) => e |. equals("err3!"); + }, + _ => () + ); }); test("tapOk", () => { @@ -135,16 +206,22 @@ describe("Future Belt.Result", () => { Belt.Result.Ok(10) |. Future.value |. Future.tapOk(n => x := x^ + n) - |. Future.get(_ => { - x^ |. equals(11); - }); + |. Future.get( + _ => { + x^ |. equals(11); + }, + _ => () + ); Belt.Result.Error(10) |. Future.value |. Future.tapOk(n => y := y^ + n) - |. Future.get(_ => { - y^ |. equals(1); - }); + |. Future.get( + _ => { + y^ |. equals(1); + }, + _ => () + ); }); test("tapError", () => { @@ -154,16 +231,22 @@ describe("Future Belt.Result", () => { Belt.Result.Ok(10) |. Future.value |. Future.tapError(n => x := x^ + n) - |. Future.get(_ => { - x^ |. equals(1); - }); + |. Future.get( + _ => { + x^ |. equals(1); + }, + _ => () + ); Belt.Result.Error(10) |. Future.value |. Future.tapError(n => y := y^ + n) - |. Future.get(_ => { - y^ |. equals(11); - }); + |. Future.get( + _ => { + y^ |. equals(11); + }, + _ => () + ); }); }); From 677d05bc32517e901fd6554d329248a5da469701 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Guyon?= Date: Thu, 24 May 2018 19:23:14 +0200 Subject: [PATCH 2/4] Reject the future when a callback raises --- src/Future.re | 48 ++++++++++++++++++++++++--------------------- tests/TestFuture.re | 21 ++++++++++++++++++++ 2 files changed, 47 insertions(+), 22 deletions(-) diff --git a/src/Future.re b/src/Future.re index e803e17..6b3ea06 100644 --- a/src/Future.re +++ b/src/Future.re @@ -9,28 +9,32 @@ let make = (resolver) => { let failureCallbacks = ref([]); let data = ref(None); - resolver( - result => switch(data^) { - | None => - data := Some(Result.Ok(result)); - successCallbacks^ |. List.reverse |. List.forEach(cb => cb(result)); - /* Clean up memory usage */ - successCallbacks := []; - failureCallbacks := [] - | Some(_) => - () /* Do nothing; theoretically not possible */ - }, - error => switch (data^) { - | None => - data := Some(Result.Error(error)); - failureCallbacks^ |. List.reverse |. List.forEach(cb => cb(error)); - - successCallbacks := []; - failureCallbacks := [] - | Some(_) => - () - } - ); + try ( + resolver( + result => switch(data^) { + | None => + data := Some(Result.Ok(result)); + successCallbacks^ |. List.reverse |. List.forEach(cb => cb(result)); + /* Clean up memory usage */ + successCallbacks := []; + failureCallbacks := [] + | Some(_) => + () /* Do nothing; theoretically not possible */ + }, + error => switch (data^) { + | None => + data := Some(Result.Error(error)); + failureCallbacks^ |. List.reverse |. List.forEach(cb => cb(error)); + + successCallbacks := []; + failureCallbacks := [] + | Some(_) => + () + } + ) + ) { + | error => data := Some(Error(error)) + }; Future((resolve, reject) => switch(data^) { | Some(Ok(result)) => resolve(result) diff --git a/tests/TestFuture.re b/tests/TestFuture.re index bfc4764..efcbe9a 100644 --- a/tests/TestFuture.re +++ b/tests/TestFuture.re @@ -95,6 +95,27 @@ describe("Future", () => { ); }); + test("raising resolver", () => { + Future.make((_resolve, _reject) => raise(Err("one"))) + |. Future.get( + _ => (), + e => { + e |. deepEquals(Err("one")); + } + ) + }); + + test("raising callback", () => { + Future.value(59) + |. Future.map(_ => raise(Err("one"))) + |. Future.get( + _ => (), + e => { + e |. deepEquals(Err("one")); + } + ) + }); + test("multiple gets", () => { let count = ref(0); let future = Future.make((resolve, _reject) => { From 4dd2c67525de17b4ebcac27d1f6d1a42fde3d2e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Guyon?= Date: Thu, 24 May 2018 19:34:09 +0200 Subject: [PATCH 3/4] Fix tests with branching --- tests/TestFuture.re | 80 +++++++++++++++++++++++++++++++++------------ 1 file changed, 60 insertions(+), 20 deletions(-) diff --git a/tests/TestFuture.re b/tests/TestFuture.re index efcbe9a..35b8768 100644 --- a/tests/TestFuture.re +++ b/tests/TestFuture.re @@ -23,7 +23,9 @@ describe("Future", () => { s => { s |. equals("one!"); }, - _ => () + _ => { + 1 |. equals(2); + } ) }); @@ -34,7 +36,9 @@ describe("Future", () => { | _ => assert false }) |. Future.get( - _ => (), + _ => { + 1 |. equals(2); + }, err => { err |. deepEquals(Err("one!")); } @@ -50,7 +54,9 @@ describe("Future", () => { s |. equals("20!"); done_(); }, - _ => () + _ => { + 1 |. equals(2); + } ); }); @@ -61,7 +67,9 @@ describe("Future", () => { | _ => assert false }) |. Future.get( - _ => (), + _ => { + 1 |. equals(2); + }, error => { error |. deepEquals(Err("20!")); done_(); @@ -80,7 +88,9 @@ describe("Future", () => { n |. equals(90); v^ |. equals(100); }, - _ => () + _ => { + 1 |. equals(2); + } ); }); @@ -91,14 +101,18 @@ describe("Future", () => { n => { n |. equals(60); }, - _ => () + _ => { + 1 |. equals(2); + } ); }); test("raising resolver", () => { Future.make((_resolve, _reject) => raise(Err("one"))) |. Future.get( - _ => (), + _ => { + 1 |. equals(2); + }, e => { e |. deepEquals(Err("one")); } @@ -109,7 +123,9 @@ describe("Future", () => { Future.value(59) |. Future.map(_ => raise(Err("one"))) |. Future.get( - _ => (), + _ => { + 1 |. equals(2); + }, e => { e |. deepEquals(Err("one")); } @@ -128,7 +144,9 @@ describe("Future", () => { c => { c |. equals(1); }, - _ => () + _ => { + 1 |. equals(2); + } ); count^ |. equals(1); @@ -136,7 +154,9 @@ describe("Future", () => { c => { c |. equals(1); }, - _ => () + _ => { + 1 |. equals(2); + } ); count^ |. equals(1); }); @@ -154,7 +174,9 @@ describe("Future", () => { _ => { count^ |. equals(~m="Runs after previous future", 1); }, - _ => () + _ => { + 1 |. equals(2); + } ); count^ |. equals(~m="Callback is async (2)", 0); @@ -162,7 +184,9 @@ describe("Future", () => { _ => { count^ |. equals(~m="Previous future only runs once", 1); }, - _ => () + _ => { + 1 |. equals(2); + } ); count^ |. equals(0, ~m="Callback is async (3)"); @@ -182,7 +206,9 @@ describe("Future Belt.Result", () => { r => { Belt.Result.getExn(r) |. equals("two!"); }, - _ => () + _ => { + 1 |. equals(2); + } ); Belt.Result.Error("err2") @@ -193,7 +219,9 @@ describe("Future Belt.Result", () => { | Ok(_) => raise(TestError("shouldn't be possible")) | Error(e) => e |. equals("err2"); }, - _ => () + _ => { + 1 |. equals(2); + } ); }); @@ -205,7 +233,9 @@ describe("Future Belt.Result", () => { r => { Belt.Result.getExn(r) |. equals("three"); }, - _ => () + _ => { + 1 |. equals(2); + } ); Belt.Result.Error("err3") @@ -216,7 +246,9 @@ describe("Future Belt.Result", () => { | Ok(_) => raise(TestError("shouldn't be possible")) | Error(e) => e |. equals("err3!"); }, - _ => () + _ => { + 1 |. equals(2); + } ); }); @@ -231,7 +263,9 @@ describe("Future Belt.Result", () => { _ => { x^ |. equals(11); }, - _ => () + _ => { + 1 |. equals(2); + } ); Belt.Result.Error(10) @@ -241,7 +275,9 @@ describe("Future Belt.Result", () => { _ => { y^ |. equals(1); }, - _ => () + _ => { + 1 |. equals(2); + } ); }); @@ -256,7 +292,9 @@ describe("Future Belt.Result", () => { _ => { x^ |. equals(1); }, - _ => () + _ => { + 1 |. equals(2); + } ); Belt.Result.Error(10) @@ -266,7 +304,9 @@ describe("Future Belt.Result", () => { _ => { y^ |. equals(11); }, - _ => () + _ => { + 1 |. equals(2); + } ); }); From d5fcde6bcc72ff0a80747971837d19bc9f923045 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Guyon?= Date: Thu, 24 May 2018 19:56:02 +0200 Subject: [PATCH 4/4] Fix bug when a callback raises --- src/Future.re | 37 ++++++++++++++++++++++------------ tests/TestFuture.re | 48 +++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 70 insertions(+), 15 deletions(-) diff --git a/src/Future.re b/src/Future.re index 6b3ea06..30703b0 100644 --- a/src/Future.re +++ b/src/Future.re @@ -50,36 +50,47 @@ let value = (x) => make((resolve, _reject) => resolve(x)); let error = (e) => make((_resolve, reject) => reject(e)); -let map = (Future(get), f) => make(resolve => { - get(result => resolve(f(result))) +let map = (Future(get), f) => make((resolve, reject) => { + get( + result => switch(f(result)) { + | value => resolve(value) + | exception error => reject(error) + }, + reject + ) }); let flatMap = (Future(get), f) => make((resolve, reject) => { get( - result => { - let Future(get2) = f(result); - get2(resolve, reject) + result => switch(f(result)) { + | Future(get2) => get2(resolve, reject) + | exception error => reject(error) }, reject ) }); -let tap = (Future(get) as future, f) => { - get(f, _error => ()); - future -}; +let tap = (Future(get), f) => make((resolve, reject) => { + get( + result => switch (f(result)) { + | () => resolve(result) + | exception error => reject(error) + }, + reject + ) +}); let catch = (Future(get), f) => make((resolve, reject) => { get( resolve, - error => { - let Future(get2) = f(error); - get2(resolve, reject) + error => switch (f(error)) { + | Future(get2) => get2(resolve, reject) + | exception error => reject(error) } ) }); -let get = (Future(getFn), f) => getFn(f); +let get = (Future(getFn), resolve, reject) => getFn(resolve, reject); /* * * Future Belt.Result convenience functions, diff --git a/tests/TestFuture.re b/tests/TestFuture.re index 35b8768..d07f2d6 100644 --- a/tests/TestFuture.re +++ b/tests/TestFuture.re @@ -119,8 +119,8 @@ describe("Future", () => { ) }); - test("raising callback", () => { - Future.value(59) + testAsync("raising map", done_ => { + delay(20, () => 59) |. Future.map(_ => raise(Err("one"))) |. Future.get( _ => { @@ -128,6 +128,50 @@ describe("Future", () => { }, e => { e |. deepEquals(Err("one")); + done_(); + } + ) + }); + + testAsync("raising flatMap", done_ => { + delay(20, () => 59) + |. Future.flatMap(_ => raise(Err("one"))) + |. Future.get( + _ => { + 1 |. equals(2); + }, + e => { + e |. deepEquals(Err("one")); + done_(); + } + ) + }); + + testAsync("raising tap", done_ => { + delay(20, () => 59) + |. Future.tap(_ => raise(Err("one"))) + |. Future.get( + _ => { + 1 |. equals(2); + }, + e => { + e |. deepEquals(Err("one")); + done_(); + } + ) + }); + + testAsync("raising catch", done_ => { + delay(20, () => 59) + |. Future.tap(_ => assert false) + |. Future.catch(_ => raise(Err("one"))) + |. Future.get( + _ => { + 1 |. equals(2); + }, + e => { + e |. deepEquals(Err("one")); + done_(); } ) });