Skip to content

Commit 8ab1cc5

Browse files
viviveeveeviviveevee
authored
feat: display output of failed builds (#125)
1 parent 28669fa commit 8ab1cc5

File tree

11 files changed

+546
-166
lines changed

11 files changed

+546
-166
lines changed

‎crates/icp-cli/src/commands/build/mod.rs‎

Lines changed: 89 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::collections::HashMap;
22

3-
use anyhow::Context as _;
3+
use anyhow::{Context as _, anyhow};
44
use camino_tempfile::tempdir;
55
use clap::Parser;
66
use futures::{StreamExt, stream::FuturesOrdered};
@@ -9,10 +9,7 @@ use icp::{
99
fs::read,
1010
};
1111

12-
use crate::{
13-
commands::Context,
14-
progress::{ProgressManager, ScriptProgressHandler},
15-
};
12+
use crate::{commands::Context, progress::ProgressManager};
1613

1714
#[derive(Parser, Debug)]
1815
pub struct Cmd {
@@ -37,6 +34,9 @@ pub enum CommandError {
3734
#[error("failed to store build artifact")]
3835
ArtifactStore,
3936

37+
#[error("failed to join build output")]
38+
JoinError(#[from] tokio::task::JoinError),
39+
4040
#[error(transparent)]
4141
Unexpected(#[from] anyhow::Error),
4242
}
@@ -77,83 +77,105 @@ pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> {
7777
// Iterate through each resolved canister and trigger its build process.
7878
for (_, (canister_path, c)) in cs {
7979
// Create progress bar with standard configuration
80-
let pb = progress_manager.create_progress_bar(&c.name);
80+
let mut pb = progress_manager.create_multi_step_progress_bar(&c.name, "Build");
8181

8282
// Create an async closure that handles the build process for this specific canister
83-
let build_fn = {
83+
let fut = {
8484
let c = c.clone();
85-
let pb = pb.clone();
8685

8786
async move {
88-
// Create a temporary directory for build artifacts
89-
let build_dir =
90-
tempdir().context("failed to create a temporary build directory")?;
91-
92-
// Prepare a path for our output wasm
93-
let wasm_output_path = build_dir.path().join("out.wasm");
94-
95-
let step_count = c.build.steps.len();
96-
for (i, step) in c.build.steps.iter().enumerate() {
97-
// Indicate to user the current step being executed
98-
let current_step = i + 1;
99-
let pb_hdr = format!("Building: {step} {current_step} of {step_count}");
100-
101-
let script_handler = ScriptProgressHandler::new(pb.clone(), pb_hdr.clone());
102-
103-
// Setup script progress handling and receiver join handle
104-
let (tx, rx) = script_handler.setup_output_handler();
105-
106-
// Perform build step
107-
ctx.builder
108-
.build(
109-
step, // step
110-
&Params {
111-
path: canister_path.to_owned(),
112-
output: wasm_output_path.to_owned(),
113-
},
114-
Some(tx),
115-
)
116-
.await?;
117-
118-
// Ensure background receiver drains all messages
119-
let _ = rx.await;
87+
// Define the build logic
88+
let build_result = async {
89+
// Create a temporary directory for build artifacts
90+
let build_dir =
91+
tempdir().context("failed to create a temporary build directory")?;
92+
93+
// Prepare a path for our output wasm
94+
let wasm_output_path = build_dir.path().join("out.wasm");
95+
96+
let step_count = c.build.steps.len();
97+
for (i, step) in c.build.steps.iter().enumerate() {
98+
// Indicate to user the current step being executed
99+
let current_step = i + 1;
100+
let pb_hdr = format!("\nBuilding: {step} {current_step} of {step_count}");
101+
let tx = pb.begin_step(pb_hdr);
102+
103+
// Perform build step
104+
let build_result = ctx
105+
.builder
106+
.build(
107+
step, // step
108+
&Params {
109+
path: canister_path.to_owned(),
110+
output: wasm_output_path.to_owned(),
111+
},
112+
Some(tx),
113+
)
114+
.await;
115+
116+
// Ensure background receiver drains all messages
117+
pb.end_step().await;
118+
119+
if let Err(e) = build_result {
120+
return Err(CommandError::Build(e));
121+
}
122+
}
123+
124+
// Verify a file exists in the wasm output path
125+
if !wasm_output_path.exists() {
126+
return Err(CommandError::MissingOutput);
127+
}
128+
129+
// Load wasm output
130+
let wasm = read(&wasm_output_path).context(CommandError::ReadOutput)?;
131+
132+
// TODO(or.ricon): Verify wasm output is valid wasm (consider using wasmparser)
133+
134+
// Save the wasm artifact
135+
ctx.artifacts
136+
.save(&c.name, &wasm)
137+
.context(CommandError::ArtifactStore)?;
138+
139+
Ok::<_, CommandError>(())
120140
}
121-
122-
// Verify a file exists in the wasm output path
123-
if !wasm_output_path.exists() {
124-
return Err(CommandError::MissingOutput);
141+
.await;
142+
143+
// Execute with progress tracking for final state
144+
let result = ProgressManager::execute_with_progress(
145+
&pb,
146+
async { build_result },
147+
|| "Built successfully".to_string(),
148+
|err| format!("Failed to build canister: {err}"),
149+
)
150+
.await;
151+
152+
// After progress bar is finished, dump the output if build failed
153+
if let Err(e) = &result {
154+
pb.dump_output(ctx);
155+
let _ = ctx
156+
.term
157+
.write_line(&format!("Failed to build canister: {e}"));
125158
}
126159

127-
// Load wasm output
128-
let wasm = read(&wasm_output_path).context(CommandError::ReadOutput)?;
129-
130-
// TODO(or.ricon): Verify wasm output is valid wasm (consider using wasmparser)
131-
132-
// Save the wasm artifact
133-
ctx.artifacts
134-
.save(&c.name, &wasm)
135-
.context(CommandError::ArtifactStore)?;
136-
137-
Ok::<_, CommandError>(())
160+
result
138161
}
139162
};
140163

141-
futs.push_back(async move {
142-
// Execute the build function with progress tracking
143-
ProgressManager::execute_with_progress(
144-
pb,
145-
build_fn,
146-
|| "Built successfully".to_string(),
147-
|err| format!("Failed to build canister: {err}"),
148-
)
149-
.await
150-
});
164+
futs.push_back(fut);
151165
}
152166

153167
// Consume the set of futures and abort if an error occurs
168+
let mut found_error = false;
154169
while let Some(res) = futs.next().await {
155-
// TODO(or.ricon): Handle canister build failures
156-
res?;
170+
if res.is_err() {
171+
found_error = true;
172+
}
173+
}
174+
175+
if found_error {
176+
return Err(CommandError::Unexpected(anyhow!(
177+
"One or more canisters failed to build"
178+
)));
157179
}
158180

159181
Ok(())

‎crates/icp-cli/src/commands/canister/binding_env_vars.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> {
233233
futs.push_back(async move {
234234
// Execute the install function with progress tracking
235235
ProgressManager::execute_with_progress(
236-
pb,
236+
&pb,
237237
settings_fn,
238238
|| "Environment variables updated successfully".to_string(),
239239
|err| format!("Failed to update environment variables: {err}"),

‎crates/icp-cli/src/commands/canister/create.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> {
347347
futs.push_back(async move {
348348
// Execute the create function with custom progress tracking
349349
let mut result = ProgressManager::execute_with_custom_progress(
350-
pb,
350+
&pb,
351351
create_fn,
352352
|| "Created successfully".to_string(),
353353
|err| match err {

‎crates/icp-cli/src/commands/canister/install.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> {
200200
futs.push_back(async move {
201201
// Execute the install function with progress tracking
202202
ProgressManager::execute_with_progress(
203-
pb,
203+
&pb,
204204
install_fn,
205205
|| "Installed successfully".to_string(),
206206
|err| format!("Failed to install canister: {err}"),

‎crates/icp-cli/src/commands/sync/mod.rs‎

Lines changed: 69 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use std::collections::HashMap;
22

3+
use anyhow::anyhow;
34
use clap::Parser;
45
use futures::{StreamExt, stream::FuturesOrdered};
56
use icp::{
@@ -11,7 +12,7 @@ use icp::{
1112
use crate::{
1213
commands::Context,
1314
options::{EnvironmentOpt, IdentityOpt},
14-
progress::{ProgressManager, ScriptProgressHandler},
15+
progress::ProgressManager,
1516
store_id::{Key, LookupError},
1617
};
1718

@@ -130,7 +131,7 @@ pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> {
130131
// Iterate through each resolved canister and trigger its sync process.
131132
for (_, (canister_path, c)) in cs {
132133
// Create progress bar with standard configuration
133-
let pb = progress_manager.create_progress_bar(&c.name);
134+
let mut pb = progress_manager.create_multi_step_progress_bar(&c.name, "Sync");
134135

135136
// Get canister principal ID
136137
let cid = ctx.ids.lookup(&Key {
@@ -140,57 +141,83 @@ pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> {
140141
})?;
141142

142143
// Create an async closure that handles the sync process for this specific canister
143-
let sync_fn = {
144-
let pb = pb.clone();
144+
let fut = {
145145
let agent = agent.clone();
146+
let c = c.clone();
146147

147148
async move {
148-
for step in &c.sync.steps {
149-
// Indicate to user the current step being executed
150-
let pb_hdr = format!("Syncing: {step}");
151-
152-
let script_handler = ScriptProgressHandler::new(pb.clone(), pb_hdr.clone());
153-
154-
// Setup script progress handling and receiver join handle
155-
let (tx, rx) = script_handler.setup_output_handler();
156-
157-
// Execute step
158-
ctx.syncer
159-
.sync(
160-
step, // step
161-
&Params {
162-
path: canister_path.to_owned(),
163-
cid: cid.to_owned(),
164-
},
165-
&agent,
166-
Some(tx),
167-
)
168-
.await?;
169-
170-
// Ensure background receiver drains all messages
171-
let _ = rx.await;
149+
// Define the sync logic
150+
let sync_result = async {
151+
let step_count = c.sync.steps.len();
152+
for (i, step) in c.sync.steps.iter().enumerate() {
153+
// Indicate to user the current step being executed
154+
let current_step = i + 1;
155+
let pb_hdr = format!("\nSyncing: {step} {current_step} of {step_count}");
156+
157+
let tx = pb.begin_step(pb_hdr);
158+
159+
// Execute step
160+
let sync_result = ctx
161+
.syncer
162+
.sync(
163+
step, // step
164+
&Params {
165+
path: canister_path.to_owned(),
166+
cid: cid.to_owned(),
167+
},
168+
&agent,
169+
Some(tx),
170+
)
171+
.await;
172+
173+
// Ensure background receiver drains all messages
174+
pb.end_step().await;
175+
176+
if let Err(e) = sync_result {
177+
return Err(CommandError::Synchronize(e));
178+
}
179+
}
180+
181+
Ok::<_, CommandError>(())
182+
}
183+
.await;
184+
185+
// Execute with progress tracking for final state
186+
let result = ProgressManager::execute_with_progress(
187+
&pb,
188+
async { sync_result },
189+
|| format!("Synced successfully: {cid}"),
190+
|err| format!("Failed to sync canister: {err}"),
191+
)
192+
.await;
193+
194+
// After progress bar is finished, dump the output if sync failed
195+
if let Err(e) = &result {
196+
pb.dump_output(ctx);
197+
let _ = ctx
198+
.term
199+
.write_line(&format!("Failed to sync canister: {e}"));
172200
}
173201

174-
Ok::<_, CommandError>(())
202+
result
175203
}
176204
};
177205

178-
futs.push_back(async move {
179-
// Execute the sync function with progress tracking
180-
ProgressManager::execute_with_progress(
181-
pb,
182-
sync_fn,
183-
|| format!("Synced successfully: {cid}"),
184-
|err| format!("Failed to sync canister: {err}"),
185-
)
186-
.await
187-
});
206+
futs.push_back(fut);
188207
}
189208

190-
// Consume the set of futures and abort if an error occurs
209+
// Consume the set of futures and collect errors
210+
let mut found_error = false;
191211
while let Some(res) = futs.next().await {
192-
// TODO(or.ricon): Handle canister sync failures
193-
res?;
212+
if res.is_err() {
213+
found_error = true;
214+
}
215+
}
216+
217+
if found_error {
218+
return Err(CommandError::Synchronize(SynchronizeError::Unexpected(
219+
anyhow!("One or more canisters failed to sync"),
220+
)));
194221
}
195222

196223
Ok(())

0 commit comments

Comments
 (0)