From 01601f5be3bfa559204d57670c96e356be822e6c Mon Sep 17 00:00:00 2001 From: Gal Schlezinger Date: Mon, 30 Dec 2019 10:37:20 +0200 Subject: [PATCH] Fix #181 by dropping `exit(1)` (#182) Seems that Lwt 5 throws a weird error when using `exit(1)` inside a promise. So instead of exiting in the command, making all executable modules a function that returns a `result((), status_code)` is a better fit. Relates to: https://github.com/ocsigen/lwt/issues/758 --- executable/Alias.re | 26 ++++++++++++------------ executable/Env.re | 2 +- executable/FnmApp.re | 44 ++++++++++++++++++++++++---------------- executable/Install.re | 8 ++++---- executable/ListLocal.re | 5 +++-- executable/ListRemote.re | 2 +- executable/Uninstall.re | 4 ++-- executable/Use.re | 13 +++++++----- 8 files changed, 58 insertions(+), 46 deletions(-) diff --git a/executable/Alias.re b/executable/Alias.re index 27c3524..8484245 100644 --- a/executable/Alias.re +++ b/executable/Alias.re @@ -16,19 +16,19 @@ let run = (~name, ~version) => { versionPath , ); - exit(1); - }; - - Logger.info( - - "Aliasing " - name - " to " - version - , - ); + Lwt.return_error(1); + } else { + Logger.info( + + "Aliasing " + name + " to " + version + , + ); - let%lwt () = Versions.Aliases.set(~alias=name, ~versionPath); + let%lwt () = Versions.Aliases.set(~alias=name, ~versionPath); - Lwt.return(); + Lwt.return_ok(); + }; }; diff --git a/executable/Env.re b/executable/Env.re index 792890f..a944294 100644 --- a/executable/Env.re +++ b/executable/Env.re @@ -158,5 +158,5 @@ let run = printUseOnCd(~shell) |> Console.log; }; - Lwt.return(); + Lwt.return_ok(); }; diff --git a/executable/FnmApp.re b/executable/FnmApp.re index 10da2ae..26d6818 100644 --- a/executable/FnmApp.re +++ b/executable/FnmApp.re @@ -1,14 +1,23 @@ let version = Fnm.Fnm__Package.version; +let runCmd = lwt => { + lwt + |> Lwt_main.run + |> ( + fun + | Error(err_code) => exit(err_code) + | Ok () => () + ); +}; + module Commands = { - let use = (version, quiet) => Lwt_main.run(Use.run(~version, ~quiet)); - let alias = (version, name) => Lwt_main.run(Alias.run(~name, ~version)); - let default = version => - Lwt_main.run(Alias.run(~name="default", ~version)); - let listRemote = () => Lwt_main.run(ListRemote.run()); - let listLocal = () => Lwt_main.run(ListLocal.run()); - let install = version => Lwt_main.run(Install.run(~version)); - let uninstall = version => Lwt_main.run(Uninstall.run(~version)); + let use = (version, quiet) => Use.run(~version, ~quiet) |> runCmd; + let alias = (version, name) => Alias.run(~name, ~version) |> runCmd; + let default = version => Alias.run(~name="default", ~version) |> runCmd; + let listRemote = () => ListRemote.run() |> runCmd; + let listLocal = () => ListLocal.run() |> runCmd; + let install = version => Install.run(~version) |> runCmd; + let uninstall = version => Uninstall.run(~version) |> runCmd; let env = ( isFishShell, @@ -19,16 +28,15 @@ module Commands = { useOnCd, logLevel, ) => - Lwt_main.run( - Env.run( - ~forceShell=Fnm.System.Shell.(isFishShell ? Some(Fish) : shell), - ~multishell=isMultishell, - ~nodeDistMirror, - ~fnmDir, - ~useOnCd, - ~logLevel, - ), - ); + Env.run( + ~forceShell=Fnm.System.Shell.(isFishShell ? Some(Fish) : shell), + ~multishell=isMultishell, + ~nodeDistMirror, + ~fnmDir, + ~useOnCd, + ~logLevel, + ) + |> runCmd; }; open Cmdliner; diff --git a/executable/Install.re b/executable/Install.re index 5376ae4..64bc9e6 100644 --- a/executable/Install.re +++ b/executable/Install.re @@ -123,7 +123,7 @@ let main = (~version as versionName) => { Lwt.return_unit; }; - Lwt.return(); + Lwt.return_ok(); }; let run = (~version) => @@ -139,7 +139,7 @@ let run = (~version) => {System.NodeArch.toString(arch)} , ); - exit(1); + Lwt.return_error(1); | Versions.Version_not_found(version) => Logger.error( @@ -148,7 +148,7 @@ let run = (~version) => " not found!" , ); - exit(1); + Lwt.return_error(1); | VersionListingLts.Problem_with_finding_latest_lts( VersionListingLts.Cant_find_latest_lts, ) => @@ -163,5 +163,5 @@ let run = (~version) => reason , ); - exit(1); + Lwt.return_error(1); }; diff --git a/executable/ListLocal.re b/executable/ListLocal.re index 0d88ee4..e0b9b30 100644 --- a/executable/ListLocal.re +++ b/executable/ListLocal.re @@ -31,12 +31,13 @@ let main = () => Console.log( "* " {version.name} aliases ); }); - Lwt.return(); + Lwt.return_ok(); } ); let run = () => try%lwt(main()) { | Cant_read_local_versions => - Console.log("No versions installed!") |> Lwt.return + Console.log("No versions installed!"); + Lwt.return_ok(); }; diff --git a/executable/ListRemote.re b/executable/ListRemote.re index 5ea8e0b..d9de1a6 100644 --- a/executable/ListRemote.re +++ b/executable/ListRemote.re @@ -22,5 +22,5 @@ let run = () => { Console.log( str ); }); - Lwt.return(); + Lwt.return_ok(); }; diff --git a/executable/Uninstall.re b/executable/Uninstall.re index fb99eba..e5b0f98 100644 --- a/executable/Uninstall.re +++ b/executable/Uninstall.re @@ -17,7 +17,7 @@ let run = (~version) => { " is not installed." , ); - exit(1); + Lwt.return_error(1); | Some(installedVersion) => Logger.debug( @@ -37,6 +37,6 @@ let run = (~version) => { " has correctly been removed." , ); - Lwt.return_unit; + Lwt.return_ok(); }; }; diff --git a/executable/Use.re b/executable/Use.re index e48adcc..25e4680 100644 --- a/executable/Use.re +++ b/executable/Use.re @@ -92,7 +92,8 @@ let main = (~version as providedVersion, ~quiet) => { | Some(version) => Lwt.return(version) | None => Dotfiles.getVersion() }; - switchVersion(~version, ~quiet); + let%lwt () = switchVersion(~version, ~quiet); + Lwt.return_ok(); }; let rec askIfInstall = (~version, ~quiet, retry) => { @@ -107,7 +108,9 @@ let rec askIfInstall = (~version, ~quiet, retry) => { let%lwt _ = Install.run(~version); retry(~version, ~quiet); | "N" - | "n" => Lwt_io.write_line(Lwt_io.stderr, "not installing!") + | "n" => + let%lwt () = Lwt_io.write_line(Lwt_io.stderr, "not installing!"); + Lwt.return_ok(); | _ => let%lwt _ = Lwt_io.write_line(Lwt_io.stderr, "Invalid response. Please try again:"); @@ -128,7 +131,7 @@ let rec run = (~version, ~quiet) => if (Fnm.Config.FNM_INTERACTIVE_CLI.get()) { askIfInstall(~version, ~quiet, run); } else { - exit(1); + Lwt.return_error(1); }; | Dotfiles.Conflicting_Dotfiles_Found(v1, v2) => error( @@ -142,7 +145,7 @@ let rec run = (~version, ~quiet) => v2 , ); - exit(1); + Lwt.return_error(1); | Dotfiles.Version_Not_Provided => error( ~quiet, @@ -150,5 +153,5 @@ let rec run = (~version, ~quiet) => "No .nvmrc or .node-version file was found in the current directory. Please provide a version number." , ); - exit(1); + Lwt.return_error(1); };