Skip to content

Commit

Permalink
Silencing expected DataErrors (#3262)
Browse files Browse the repository at this point in the history
  • Loading branch information
robertbastian authored Apr 6, 2023
1 parent cf4351f commit 3eff54d
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 14 deletions.
12 changes: 10 additions & 2 deletions experimental/segmenter/src/complex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ impl LstmPayloads {
) -> Result<Option<DataPayload<LstmDataV1Marker>>, DataError> {
match provider.load(DataRequest {
locale: &DataLocale::from(locale),
metadata: Default::default(),
metadata: {
let mut m = DataRequestMetadata::default();
m.silent = true;
m
},
}) {
Ok(response) => Ok(Some(response.take_payload()?.cast())),
Err(DataError {
Expand Down Expand Up @@ -133,7 +137,11 @@ impl Dictionary {
provider
.load(DataRequest {
locale: &DataLocale::from(locale),
metadata: Default::default(),
metadata: {
let mut m = DataRequestMetadata::default();
m.silent = true;
m
},
})?
.take_payload()
.map(DataPayload::cast)
Expand Down
15 changes: 10 additions & 5 deletions provider/adapters/src/fallback/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ).

use super::*;
use crate::helpers::result_is_err_missing_data_options;
use crate::helpers::result_is_err_missing_locale;

/// A data provider wrapper that performs locale fallback. This enables arbitrary locales to be
/// handled at runtime.
Expand Down Expand Up @@ -182,7 +182,7 @@ impl<P> LocaleFallbackProvider<P> {
fn run_fallback<F1, F2, R>(
&self,
key: DataKey,
base_req: DataRequest,
mut base_req: DataRequest,
mut f1: F1,
mut f2: F2,
) -> Result<R, DataError>
Expand All @@ -192,26 +192,31 @@ impl<P> LocaleFallbackProvider<P> {
{
let key_fallbacker = self.fallbacker.for_key(key);
let mut fallback_iterator = key_fallbacker.fallback_for(base_req.locale.clone());
let base_silent = core::mem::replace(&mut base_req.metadata.silent, true);
loop {
let result = f1(DataRequest {
locale: fallback_iterator.get(),
metadata: Default::default(),
metadata: base_req.metadata,
});
if !result_is_err_missing_data_options(&result) {
if !result_is_err_missing_locale(&result) {
return result
.map(|mut res| {
f2(&mut res).locale = Some(fallback_iterator.take());
res
})
// Log the original request rather than the fallback request
.map_err(|e| e.with_req(key, base_req));
.map_err(|e| {
base_req.metadata.silent = base_silent;
e.with_req(key, base_req)
});
}
// If we just checked und, break out of the loop.
if fallback_iterator.get().is_empty() {
break;
}
fallback_iterator.step();
}
base_req.metadata.silent = base_silent;
Err(DataErrorKind::MissingLocale.with_req(key, base_req))
}
}
Expand Down
2 changes: 1 addition & 1 deletion provider/adapters/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use icu_provider::prelude::*;

pub(crate) fn result_is_err_missing_data_options<T>(result: &Result<T, DataError>) -> bool {
pub(crate) fn result_is_err_missing_locale<T>(result: &Result<T, DataError>) -> bool {
matches!(
result,
Err(DataError {
Expand Down
27 changes: 22 additions & 5 deletions provider/core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ pub struct DataError {

/// Additional context, if available.
pub str_context: Option<&'static str>,

/// Whether this error was created in silent mode to not log.
pub silent: bool,
}

impl fmt::Display for DataError {
Expand Down Expand Up @@ -129,6 +132,7 @@ impl DataErrorKind {
kind: self,
key: None,
str_context: None,
silent: false,
}
}

Expand Down Expand Up @@ -165,6 +169,7 @@ impl DataError {
kind: DataErrorKind::Custom,
key: None,
str_context: Some(str_context),
silent: false,
}
}

Expand All @@ -175,6 +180,7 @@ impl DataError {
kind: self.kind,
key: Some(key),
str_context: self.str_context,
silent: self.silent,
}
}

Expand All @@ -185,6 +191,7 @@ impl DataError {
kind: self.kind,
key: self.key,
str_context: Some(context),
silent: self.silent,
}
}

Expand All @@ -199,10 +206,13 @@ impl DataError {
/// If the "log_error_context" Cargo feature is enabled, this logs the whole request. Either way,
/// it returns an error with the resource key portion of the request as context.
#[cfg_attr(not(feature = "log_error_context"), allow(unused_variables))]
pub fn with_req(self, key: DataKey, req: DataRequest) -> Self {
pub fn with_req(mut self, key: DataKey, req: DataRequest) -> Self {
if req.metadata.silent {
self.silent = true;
}
// Don't write out a log for MissingDataKey since there is no context to add
#[cfg(feature = "log_error_context")]
if self.kind != DataErrorKind::MissingDataKey {
if !self.silent && self.kind != DataErrorKind::MissingDataKey {
log::warn!("{} (key: {}, request: {})", self, key, req);
}
self.with_key(key)
Expand All @@ -216,7 +226,9 @@ impl DataError {
#[cfg_attr(not(feature = "log_error_context"), allow(unused_variables))]
pub fn with_path_context<P: AsRef<std::path::Path> + ?Sized>(self, path: &P) -> Self {
#[cfg(feature = "log_error_context")]
log::warn!("{} (path: {:?})", self, path.as_ref());
if !self.silent {
log::warn!("{} (path: {:?})", self, path.as_ref());
}
self
}

Expand All @@ -228,7 +240,9 @@ impl DataError {
#[inline]
pub fn with_display_context<D: fmt::Display + ?Sized>(self, context: &D) -> Self {
#[cfg(feature = "log_error_context")]
log::warn!("{}: {}", self, context);
if !self.silent {
log::warn!("{}: {}", self, context);
}
self
}

Expand All @@ -240,7 +254,9 @@ impl DataError {
#[inline]
pub fn with_debug_context<D: fmt::Debug + ?Sized>(self, context: &D) -> Self {
#[cfg(feature = "log_error_context")]
log::warn!("{}: {:?}", self, context);
if !self.silent {
log::warn!("{}: {:?}", self, context);
}
self
}

Expand All @@ -250,6 +266,7 @@ impl DataError {
kind: DataErrorKind::MismatchedType(core::any::type_name::<T>()),
key: None,
str_context: None,
silent: false,
}
}
}
Expand Down
1 change: 1 addition & 0 deletions provider/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ pub mod prelude {
pub use crate::marker::KeyedDataMarker;
pub use crate::request::DataLocale;
pub use crate::request::DataRequest;
pub use crate::request::DataRequestMetadata;
pub use crate::response::DataPayload;
pub use crate::response::DataResponse;
pub use crate::response::DataResponseMetadata;
Expand Down
5 changes: 4 additions & 1 deletion provider/core/src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ impl fmt::Display for DataRequest<'_> {
/// for tuning locale fallback, buffer layout, and so forth.
#[derive(Default, Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
#[non_exhaustive]
pub struct DataRequestMetadata;
pub struct DataRequestMetadata {
/// Silent requests do not log errors. This can be used for exploratory querying, such as fallbacks.
pub silent: bool,
}

/// The main locale type used by the ICU4X data provider.
///
Expand Down

0 comments on commit 3eff54d

Please sign in to comment.