refactor: add more errors and better details to the build command

dfx/src/commands/build.rs

@@ -11,6 +11,85 @@ pub fn construct() -> App<'static, 'static> {
         .arg(Arg::with_name("canister").help("The canister name to build."))
 }
 
+/// Compile an actorscript file.
+fn actorscript_compile<T: BinaryResolverEnv>(
+    env: &T,
+    input_path: &Path,
+    output_path: &Path,
+    profile: Option<Profile>,
+) -> DfxResult {
+    // Invoke the compiler in debug (development) or release mode, based on the current profile:
+    let arg_profile = match profile {
+        Some(Profile::Release) => "--release",
+        _ => "--debug",
+    };
+
+    let as_rts_path = env.get_binary_command_path("as-rts.wasm")?;
+    let output = env
+        .get_binary_command("asc")?
+        .env("ASC_RTS", as_rts_path.as_path())
+        .arg(&input_path)
+        .arg(arg_profile)
+        .arg("-o")
+        .arg(&output_path)
+        .output()?;
+
+    if !output.status.success() {
+        Err(DfxError::BuildError(
+            BuildErrorKind::ActorScriptCompilerError(
+                // We choose to join the strings and not the vector in case there is a weird
+                // incorrect character at the end of stdout.
+                String::from_utf8_lossy(&output.stdout).to_string()
+                    + &String::from_utf8_lossy(&output.stderr),
+            ),
+        ))
+    } else {
+        Ok(())
+    }
+}
+
+fn didl_compile<T: BinaryResolverEnv>(env: &T, input_path: &Path, output_path: &Path) -> DfxResult {
+    let output = env
+        .get_binary_command("asc")?
+        .arg("--idl")
+        .arg(&input_path)
+        .arg("-o")
+        .arg(&output_path)
+        .output()?;
+
+    if !output.status.success() {
+        Err(DfxError::BuildError(BuildErrorKind::IdlGenerationError(
+            String::from_utf8_lossy(&output.stdout).to_string(),
+        )))
+    } else {
+        Ok(())
+    }
+}
+
+fn build_user_lib<T: BinaryResolverEnv>(
+    env: &T,
+    input_path: &Path,
+    output_path: &Path,
+) -> DfxResult {
+    let output = env
+        .get_binary_command("didc")?
+        .arg("--js")
+        .arg(&input_path)
+        .arg("-o")
+        .arg(&output_path)
+        .output()?;
+
+    if !output.status.success() {
+        Err(DfxError::BuildError(
+            BuildErrorKind::UserLibGenerationError(
+                String::from_utf8_lossy(&output.stdout).to_string(),
+            ),
+        ))
+    } else {
+        Ok(())
+    }
+}
+
 fn build_file<T>(
     env: &T,
     profile: Option<Profile>,
@@ -35,54 +114,10 @@ where
         Some("as") => {
             let output_idl_path = output_path.with_extension("did");
             let output_js_path = output_path.with_extension("js");
-            let as_rts_path = env.get_binary_command_path("as-rts.wasm")?;
-
-            // invoke the compiler in debug (development) or release mode,
-            // based on the current profile:
-            let arg_profile = match profile {
-                None | Some(Profile::Debug) => "--debug",
-                Some(Profile::Release) => "--release",
-            };
-
-            if !env
-                .get_binary_command("asc")?
-                .env("ASC_RTS", as_rts_path.as_path())
-                .arg(&input_path)
-                .arg(arg_profile)
-                .arg("-o")
-                .arg(&output_wasm_path)
-                .output()?
-                .status
-                .success()
-            {
-                return Err(DfxError::BuildError(BuildErrorKind::CompilerError));
-            }
 
-            if !env
-                .get_binary_command("asc")?
-                .arg("--idl")
-                .arg(&input_path)
-                .arg("-o")
-                .arg(&output_idl_path)
-                .output()?
-                .status
-                .success()
-            {
-                return Err(DfxError::BuildError(BuildErrorKind::CompilerError));
-            }
-
-            if !env
-                .get_binary_command("didc")?
-                .arg("--js")
-                .arg(&output_idl_path)
-                .arg("-o")
-                .arg(&output_js_path)
-                .output()?
-                .status
-                .success()
-            {
-                return Err(DfxError::BuildError(BuildErrorKind::CompilerError));
-            }
+            actorscript_compile(env, &input_path, &output_wasm_path, profile)?;
+            didl_compile(env, &input_path, &output_idl_path)?;
+            build_user_lib(env, &output_idl_path, &output_js_path)?;
 
             Ok(())
         }

dfx/src/config/cache.rs

@@ -85,9 +85,7 @@ pub fn get_binary_path_from_version(version: &str, binary_name: &str) -> Result<
 
 pub fn binary_command_from_version(version: &str, name: &str) -> Result<std::process::Command> {
     let path = get_binary_path_from_version(version, name)?;
-    let mut cmd = std::process::Command::new(path);
-    cmd.stdout(std::process::Stdio::inherit());
-    cmd.stderr(std::process::Stdio::inherit());
+    let cmd = std::process::Command::new(path);
 
     Ok(cmd)
 }

dfx/src/lib/error.rs

@@ -1,14 +1,55 @@
 use ic_http_agent::{RequestIdError, RequestIdFromStringError};
+use std::fmt;
 
 #[derive(Debug)]
+/// An error happened during build.
 pub enum BuildErrorKind {
+    /// Invalid extension.
     InvalidExtension(String),
-    CompilerError,
+
+    /// A compiler error happened.
+    ActorScriptCompilerError(String),
+
+    /// An error happened during the generation of the Idl.
+    IdlGenerationError(String),
+
+    /// An error happened while generating the user library.
+    UserLibGenerationError(String),
+}
+
+impl fmt::Display for BuildErrorKind {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        use BuildErrorKind::*;
+
+        match self {
+            InvalidExtension(ext) => {
+                f.write_fmt(format_args!("Invalid extension: {}", ext))?;
+            }
+            ActorScriptCompilerError(stdout) => {
+                f.write_fmt(format_args!("ActorScript returned an error:\n{}", stdout))?;
+            }
+            IdlGenerationError(stdout) => {
+                f.write_fmt(format_args!(
+                    "IDL generation returned an error:\n{}",
+                    stdout
+                ))?;
+            }
+            UserLibGenerationError(stdout) => {
+                f.write_fmt(format_args!(
+                    "UserLib generation returned an error:\n{}",
+                    stdout
+                ))?;
+            }
+        };
+
+        Ok(())
+    }
 }
 
 // TODO: refactor this enum into a *Kind enum and a struct DfxError.
 #[derive(Debug)]
 pub enum DfxError {
+    /// An error happened during build.
     BuildError(BuildErrorKind),
     Clap(clap::Error),
     IO(std::io::Error),

dfx/src/main.rs

@@ -72,8 +72,16 @@ fn main() {
         }
     };
 
-    if let Err(err) = result {
-        println!("An error occured:\n{:#?}", err);
-        ::std::process::exit(255)
+    match result {
+        Ok(()) => {}
+        Err(DfxError::BuildError(err)) => {
+            eprintln!("Build failed. Reason:");
+            eprintln!("  {}", err);
+            std::process::exit(255)
+        }
+        Err(err) => {
+            eprintln!("An error occured:\n{:#?}", err);
+            std::process::exit(255)
+        }
     }
 }

e2e/build.bash

@@ -0,0 +1,28 @@
+#!/usr/bin/env bats
+
+load utils/_
+
+setup() {
+    # We want to work from a temporary directory, different for every test.
+    cd $(mktemp -d -t dfx-e2e-XXXXXXXX)
+    export RUST_BACKTRACE=1
+
+    dfx_new
+}
+
+teardown() {
+    # Kill the node manager, the dfx and the client. Ignore errors (ie. if processes aren't
+    # running).
+    killall dfx nodemanager client || true
+}
+
+@test "build fails on invalid actorscript" {
+    install_asset invalid_as
+    assert_command_fail dfx build
+    assert_match "syntax error"
+}
+
+@test "build succeeds on default project" {
+    assert_command dfx build
+    assert_match "Building hello..."
+}

e2e/utils/assertions.bash

@@ -23,7 +23,7 @@ assert_command() {
 assert_command_fail() {
     run "$@"
     [[ $status != 0 ]] || \
-        (  (echo "$*"; echo $output | batslib_decorate "Output") \
+        (  (echo "$*"; echo "$output" | batslib_decorate "Output") \
          | batslib_decorate "Command succeeded (should have failed)" \
          | fail)
 }
@@ -34,11 +34,11 @@ assert_command_fail() {
 #    $2 - The string to match against (output). By default it will use
 #         $output.
 assert_match() {
-    regex=$1
+    regex="$1"
     if [[ $# < 2 ]]; then
-        text=$output
+        text="$output"
     else
-        text=$2
+        text="$2"
     fi
     [[ "$text" =~ "$regex" ]] || \
         (batslib_print_kv_single_or_multi 10 "regex" "$regex" "actual" "$text" \
@@ -51,15 +51,15 @@ assert_match() {
 #    $1 - The expected value.
 #    $2 - The actual value.
 assert_eq() {
-    expected=$1
+    expected="$1"
     if [[ $# < 2 ]]; then
-        actual=$output
+        actual="$output"
     else
-        actual=$2
+        actual="$2"
     fi
 
     [[ "$actual" == "$expected" ]] || \
-        (batslib_print_kv_single_or_multi 10 "expected" $expected "actual" $actual \
+        (batslib_print_kv_single_or_multi 10 "expected" "$expected" "actual" "$actual" \
          | batslib_decorate "output does not match" \
          | fail)
 }
@@ -69,15 +69,15 @@ assert_eq() {
 #    $1 - The expected value.
 #    $2 - The actual value.
 assert_neq() {
-    expected=$1
+    expected="$1"
     if [[ $# < 2 ]]; then
-        actual=$output
+        actual="$output"
     else
-        actual=$2
+        actual="$2"
     fi
 
     [[ "$actual" != "$expected" ]] || \
-        (batslib_print_kv_single_or_multi 10 "expected" $expected "actual" $actual \
+        (batslib_print_kv_single_or_multi 10 "expected" "$expected" "actual" "$actual" \
          | batslib_decorate "output does not match" \
          | fail)
 }