Skip to content

Commit

Permalink
fix: do not log cache creation failure on readonly file system (#27794)
Browse files Browse the repository at this point in the history
  • Loading branch information
dsherret authored Jan 23, 2025
1 parent c38dbe5 commit be703b7
Showing 1 changed file with 48 additions and 16 deletions.
64 changes: 48 additions & 16 deletions cli/cache/cache_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ impl CacheDB {
config: &CacheDBConfiguration,
conn: &Connection,
version: &str,
) -> Result<(), AnyError> {
) -> Result<(), rusqlite::Error> {
let sql = config.create_combined_sql();
conn.execute_batch(&sql)?;

Expand Down Expand Up @@ -265,7 +265,7 @@ impl CacheDB {
fn open_connection_and_init(
&self,
path: Option<&Path>,
) -> Result<Connection, AnyError> {
) -> Result<Connection, rusqlite::Error> {
let conn = self.actually_open_connection(path)?;
Self::initialize_connection(self.config, &conn, self.version)?;
Ok(conn)
Expand Down Expand Up @@ -368,7 +368,9 @@ impl CacheDB {
fn open_connection(
config: &CacheDBConfiguration,
path: Option<&Path>,
open_connection_and_init: impl Fn(Option<&Path>) -> Result<Connection, AnyError>,
open_connection_and_init: impl Fn(
Option<&Path>,
) -> Result<Connection, rusqlite::Error>,
) -> Result<ConnectionState, AnyError> {
// Success on first try? We hope that this is the case.
let err = match open_connection_and_init(path) {
Expand All @@ -379,9 +381,20 @@ fn open_connection(
let Some(path) = path.as_ref() else {
// If an in-memory DB fails, that's game over
log::error!("Failed to initialize in-memory cache database.");
return Err(err);
return Err(err.into());
};

// reduce logging for readonly file system
if let rusqlite::Error::SqliteFailure(ffi_err, _) = &err {
if ffi_err.code == rusqlite::ErrorCode::ReadOnly {
log::debug!(
"Failed creating cache db. Folder readonly: {}",
path.display()
);
return handle_failure_mode(config, err, open_connection_and_init);
}
}

// ensure the parent directory exists
if let Some(parent) = path.parent() {
match std::fs::create_dir_all(parent) {
Expand Down Expand Up @@ -410,10 +423,11 @@ fn open_connection(
// Failed, try deleting it
let is_tty = std::io::stderr().is_terminal();
log::log!(
if is_tty { log::Level::Warn } else { log::Level::Trace },
"Could not initialize cache database '{}', deleting and retrying... ({err:?})",
path.to_string_lossy()
);
if is_tty { log::Level::Warn } else { log::Level::Trace },
"Could not initialize cache database '{}', deleting and retrying... ({err:?})",
path.to_string_lossy()
);

if std::fs::remove_file(path).is_ok() {
// Try a third time if we successfully deleted it
let res = open_connection_and_init(Some(path));
Expand All @@ -422,6 +436,11 @@ fn open_connection(
};
}

log_failure_mode(path, is_tty, config);
handle_failure_mode(config, err, open_connection_and_init)
}

fn log_failure_mode(path: &Path, is_tty: bool, config: &CacheDBConfiguration) {
match config.on_failure {
CacheFailure::InMemory => {
log::log!(
Expand All @@ -431,9 +450,8 @@ fn open_connection(
log::Level::Trace
},
"Failed to open cache file '{}', opening in-memory cache.",
path.to_string_lossy()
path.display()
);
Ok(ConnectionState::Connected(open_connection_and_init(None)?))
}
CacheFailure::Blackhole => {
log::log!(
Expand All @@ -443,23 +461,36 @@ fn open_connection(
log::Level::Trace
},
"Failed to open cache file '{}', performance may be degraded.",
path.to_string_lossy()
path.display()
);
Ok(ConnectionState::Blackhole)
}
CacheFailure::Error => {
log::error!(
"Failed to open cache file '{}', expect further errors.",
path.to_string_lossy()
path.display()
);
Err(err)
}
}
}

fn handle_failure_mode(
config: &CacheDBConfiguration,
err: rusqlite::Error,
open_connection_and_init: impl Fn(
Option<&Path>,
) -> Result<Connection, rusqlite::Error>,
) -> Result<ConnectionState, AnyError> {
match config.on_failure {
CacheFailure::InMemory => {
Ok(ConnectionState::Connected(open_connection_and_init(None)?))
}
CacheFailure::Blackhole => Ok(ConnectionState::Blackhole),
CacheFailure::Error => Err(err.into()),
}
}

#[cfg(test)]
mod tests {
use deno_core::anyhow::anyhow;
use test_util::TempDir;

use super::*;
Expand Down Expand Up @@ -520,7 +551,8 @@ mod tests {
let path = temp_dir.path().join("data").to_path_buf();
let state = open_connection(&TEST_DB, Some(path.as_path()), |maybe_path| {
match maybe_path {
Some(_) => Err(anyhow!("fail")),
// this error was chosen because it was an error easy to construct
Some(_) => Err(rusqlite::Error::SqliteSingleThreadedMode),
None => Ok(Connection::open_in_memory().unwrap()),
}
})
Expand Down

0 comments on commit be703b7

Please sign in to comment.