Skip to content

Commit 706e81d

Browse files
committed
fix(migration): Capture stderr from psql to prevent silent failures
The previous implementation used `Stdio::inherit` for the psql process, which meant that stderr was not captured by the application. This could cause the replication to appear to hang if psql encountered an error or printed a notice, as the application had no way of knowing the process had terminated or was waiting. This change refactors the `restore_globals` function to use `tokio::process::Command` to asynchronously execute psql and capture its stdout and stderr. - If psql exits with an error, the contents of stderr are now logged, providing clear feedback on the cause of the failure. - Non-fatal notices (e.g., "role already exists") are treated as warnings, and the process continues. This ensures that the application no longer hangs silently and provides better diagnostic information to the user.
1 parent f1bb1d9 commit 706e81d

1 file changed

Lines changed: 64 additions & 53 deletions

File tree

src/migration/restore.rs

Lines changed: 64 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
use anyhow::{Context, Result};
55
use std::process::{Command, Stdio};
66
use std::time::Duration;
7+
use tokio::process::Command as TokioCommand;
78

89
/// Restore global objects using psql
910
pub async fn restore_globals(target_url: &str, input_path: &str) -> Result<()> {
@@ -16,65 +17,75 @@ pub async fn restore_globals(target_url: &str, input_path: &str) -> Result<()> {
1617
.context("Failed to create .pgpass file for authentication")?;
1718

1819
let env_vars = parts.to_pg_env_vars();
19-
let input_path_owned = input_path.to_string();
2020

21-
// Wrap subprocess execution with retry logic
22-
let result = crate::utils::retry_subprocess_with_backoff(
23-
|| {
24-
let mut cmd = Command::new("psql");
25-
cmd.arg("--host")
26-
.arg(&parts.host)
27-
.arg("--port")
28-
.arg(parts.port.to_string())
29-
.arg("--dbname")
30-
.arg(&parts.database)
31-
.arg(format!("--file={}", input_path_owned))
32-
.arg("--quiet")
33-
.arg("-v")
34-
.arg("ON_ERROR_STOP=1") // Stop on first error for better visibility
35-
.env("PGPASSFILE", pgpass.path())
36-
.stdout(Stdio::inherit())
37-
.stderr(Stdio::inherit());
38-
39-
// Add username if specified
40-
if let Some(user) = &parts.user {
41-
cmd.arg("--username").arg(user);
42-
}
21+
let mut cmd = TokioCommand::new("psql");
22+
cmd.arg("--host")
23+
.arg(&parts.host)
24+
.arg("--port")
25+
.arg(parts.port.to_string())
26+
.arg("--dbname")
27+
.arg(&parts.database)
28+
.arg(format!("--file={}", input_path))
29+
.arg("--quiet")
30+
.arg("-v")
31+
.arg("ON_ERROR_STOP=1") // Stop on first error for better visibility
32+
.env("PGPASSFILE", pgpass.path())
33+
.stdout(Stdio::piped())
34+
.stderr(Stdio::piped());
35+
36+
// Add username if specified
37+
if let Some(user) = &parts.user {
38+
cmd.arg("--username").arg(user);
39+
}
4340

44-
// Apply query parameters as environment variables (SSL, channel_binding, etc.)
45-
for (env_var, value) in &env_vars {
46-
cmd.env(env_var, value);
47-
}
41+
// Apply query parameters as environment variables (SSL, channel_binding, etc.)
42+
for (env_var, value) in &env_vars {
43+
cmd.env(env_var, value);
44+
}
4845

49-
// Apply TCP keepalive parameters to prevent idle connection timeouts
50-
for (env_var, value) in crate::utils::get_keepalive_env_vars() {
51-
cmd.env(env_var, value);
52-
}
46+
// Apply TCP keepalive parameters to prevent idle connection timeouts
47+
for (env_var, value) in crate::utils::get_keepalive_env_vars() {
48+
cmd.env(env_var, value);
49+
}
5350

54-
cmd.status().context(
55-
"Failed to execute psql. Is PostgreSQL client installed?\n\
56-
Install with:\n\
57-
- Ubuntu/Debian: sudo apt-get install postgresql-client\n\
58-
- macOS: brew install postgresql\n\
59-
- RHEL/CentOS: sudo yum install postgresql",
60-
)
61-
},
62-
3, // Max 3 retries
63-
Duration::from_secs(1), // Start with 1 second delay
64-
"psql (restore globals)",
65-
)
66-
.await;
51+
let output = cmd.output().await.context(
52+
"Failed to execute psql. Is PostgreSQL client installed?\n\
53+
Install with:\n\
54+
- Ubuntu/Debian: sudo apt-get install postgresql-client\n\
55+
- macOS: brew install postgresql\n\
56+
- RHEL/CentOS: sudo yum install postgresql",
57+
)?;
6758

68-
// Handle result - don't fail on warnings for global objects
69-
match result {
70-
Ok(()) => {
71-
tracing::info!("✓ Global objects restored");
72-
Ok(())
59+
if output.status.success() {
60+
tracing::info!("✓ Global objects restored");
61+
// Even on success, there might be NOTICES or WARNINGS on stderr
62+
let stderr = String::from_utf8_lossy(&output.stderr);
63+
if !stderr.trim().is_empty() {
64+
tracing::debug!("psql stderr output:\n{}", stderr);
7365
}
74-
Err(e) => {
75-
tracing::warn!("⚠ Some global object restoration warnings occurred: {}", e);
76-
// Don't fail - some errors are expected (roles may already exist)
77-
Ok(())
66+
Ok(())
67+
} else {
68+
let stderr = String::from_utf8_lossy(&output.stderr);
69+
let stdout = String::from_utf8_lossy(&output.stdout);
70+
71+
// Check for common, non-fatal warnings
72+
if stderr.contains("already exists") && stderr.contains("skipping") {
73+
tracing::warn!(
74+
"⚠ Some global objects already exist, which is usually safe. Full output:\n{}",
75+
stderr
76+
);
77+
Ok(()) // Treat as success
78+
} else {
79+
// A real error occurred
80+
anyhow::bail!(
81+
"Failed to restore global objects.\n\
82+
Exit Code: {}\n\
83+
Stderr:\n{}\n\
84+
Stdout:\n{}",
85+
output.status,
86+
stderr,
87+
stdout
88+
);
7889
}
7990
}
8091
}

0 commit comments

Comments
 (0)