Skip to content

Commit e0b062e

Browse files
christollidayfacebook-github-bot
authored andcommitted
Use buck2_error instead of anyhow for soft_error
Summary: We should be able to tag soft errors for the same reason we tag normal errors. Tags are not used yet but will be logged to logview later. This would also simplify logging these to buck2_errors, if we choose to do that. This is used to pass `Deprecation` tags to soft errors in this stack, but seems useful regardless. Reviewed By: JakobDegen Differential Revision: D60694255 fbshipit-source-id: 673c100b6e9ecb8f2fabb200b2a55a7de003690e
1 parent d7bd442 commit e0b062e

File tree

23 files changed

+45
-35
lines changed

23 files changed

+45
-35
lines changed

app/buck2/src/panic.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ mod imp {
150150
pub(crate) fn write_soft_error(
151151
fb: FacebookInit,
152152
category: &str,
153-
err: &anyhow::Error,
153+
err: &buck2_error::Error,
154154
location: Location,
155155
options: StructuredErrorOptions,
156156
) {

app/buck2_action_impl/src/actions/impls/run/dep_files.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1251,7 +1251,7 @@ impl DeclaredDepFiles {
12511251
None => {
12521252
soft_error!(
12531253
"missing_dep_file",
1254-
anyhow::anyhow!("Dep file is missing at {}", dep_file_path)
1254+
anyhow::anyhow!("Dep file is missing at {}", dep_file_path).into()
12551255
)?;
12561256
return Ok(None);
12571257
}

app/buck2_build_signals/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ where
131131
.await
132132
.context("Error computing critical path");
133133
if let Err(e) = res {
134-
soft_error!("critical_path_computation_failed", e)?;
134+
soft_error!("critical_path_computation_failed", e.into())?;
135135
}
136136
result
137137
}

app/buck2_client/src/commands/debug/persist_event_logs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ fn status_from_result(res: anyhow::Result<()>) -> (Vec<String>, Option<String>,
297297
Some(categorize_error(&e).to_owned()),
298298
false,
299299
);
300-
let _unused = soft_error!(categorize_error(&e), e);
300+
let _unused = soft_error!(categorize_error(&e), e.into());
301301
status
302302
} else {
303303
(vec![], None, true)

app/buck2_configured/src/nodes/calculation.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1444,7 +1444,7 @@ impl ConfiguredTargetNodeCalculationImpl for ConfiguredTargetNodeCalculationInst
14441444
&reason.cause,
14451445
&IncompatiblePlatformReasonCause::Dependency(_)
14461446
) {
1447-
soft_error!("dep_only_incompatible", reason.to_err(), quiet: true)?;
1447+
soft_error!("dep_only_incompatible", reason.to_err().into(), quiet: true)?;
14481448
}
14491449
}
14501450
}

app/buck2_core/BUCK

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,5 +99,6 @@ rust_unittest(
9999
deps = [
100100
"fbsource//third-party/rust:anyhow",
101101
":buck2_core",
102+
"//buck2/app/buck2_error:buck2_error",
102103
],
103104
)

app/buck2_core/src/error.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use crate::env::__macro_refs::buck2_env;
2222
use crate::is_open_source;
2323

2424
type StructuredErrorHandler = Box<
25-
dyn for<'a> Fn(&'a str, &anyhow::Error, (&'a str, u32, u32), StructuredErrorOptions)
25+
dyn for<'a> Fn(&'a str, &buck2_error::Error, (&'a str, u32, u32), StructuredErrorOptions)
2626
+ Send
2727
+ Sync
2828
+ 'static,
@@ -49,7 +49,7 @@ static ALL_SOFT_ERROR_COUNTERS: Mutex<Vec<&'static AtomicUsize>> = Mutex::new(Ve
4949
///
5050
/// * The category string that will remain constant and identifies this specific soft error
5151
/// (used to report as a key).
52-
/// * The error is an `anyhow::Error` will in the future will be propagated as the error.
52+
/// * The error is an `buck2_error::Error` will in the future will be propagated as the error.
5353
///
5454
/// Soft errors from Meta internal runs can be viewed
5555
/// [in logview](https://www.internalfb.com/logview/overview/buck2).
@@ -159,12 +159,12 @@ impl Default for StructuredErrorOptions {
159159
#[doc(hidden)]
160160
pub fn handle_soft_error(
161161
category: &str,
162-
err: anyhow::Error,
162+
err: buck2_error::Error,
163163
count: &'static AtomicUsize,
164164
once: &std::sync::Once,
165165
loc: (&'static str, u32, u32),
166166
options: StructuredErrorOptions,
167-
) -> anyhow::Result<anyhow::Error> {
167+
) -> Result<buck2_error::Error, buck2_error::Error> {
168168
validate_category(category)?;
169169

170170
if cfg!(test) {
@@ -184,7 +184,9 @@ pub fn handle_soft_error(
184184
}
185185

186186
if hard_error_config()?.should_hard_error(category) {
187-
return Err(err.context("Upgraded warning to failure via $BUCK2_HARD_ERROR"));
187+
return Err(err
188+
.context("Upgraded warning to failure via $BUCK2_HARD_ERROR")
189+
.into());
188190
}
189191

190192
if is_open_source() {

app/buck2_core/src/provider/flavors.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ pub fn map_flavors(flavors: &str, full_target: &str) -> anyhow::Result<Providers
5555
// rely on the wrapping span in order to find
5656
soft_error!(
5757
"platform_flavor",
58-
anyhow::anyhow!("Platform flavor found in target: {}", full_target),
58+
anyhow::anyhow!("Platform flavor found in target: {}", full_target).into(),
5959
quiet: true
6060
)?;
6161
flavors_parts.remove(index);

app/buck2_core/tests/soft_error.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ static RESULT: Mutex<Vec<String>> = Mutex::new(Vec::new());
2121

2222
fn mock_handler(
2323
category: &str,
24-
err: &anyhow::Error,
24+
err: &buck2_error::Error,
2525
loc: (&str, u32, u32),
2626
options: StructuredErrorOptions,
2727
) {
@@ -57,7 +57,7 @@ fn test_soft_error() {
5757
let before_error_line = line!();
5858
let _ignore_hard_error = soft_error!(
5959
"test_logged_soft_error",
60-
anyhow::anyhow!("Should be logged")
60+
anyhow::anyhow!("Should be logged").into()
6161
);
6262
assert_eq!(
6363
Some(&format!(
@@ -79,7 +79,7 @@ fn test_reset_counters() {
7979
assert_eq!(0, RESULT.lock().unwrap().len(), "Sanity check");
8080

8181
for _ in 0..100 {
82-
let _ignore = soft_error!("test_reset_counters", anyhow::anyhow!("Message"));
82+
let _ignore = soft_error!("test_reset_counters", anyhow::anyhow!("Message").into());
8383
}
8484

8585
assert_eq!(
@@ -91,7 +91,7 @@ fn test_reset_counters() {
9191
reset_soft_error_counters();
9292

9393
for _ in 0..100 {
94-
let _ignore = soft_error!("test_reset_counters", anyhow::anyhow!("Message"));
94+
let _ignore = soft_error!("test_reset_counters", anyhow::anyhow!("Message").into());
9595
}
9696

9797
assert_eq!(

app/buck2_execute/src/re/uploader.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ impl Uploader {
275275
"{} missing (origin: {})",
276276
file.digest,
277277
info.origin.as_display_for_not_found(),
278-
),
278+
).into(),
279279
daemon_in_memory_state_is_corrupted: true,
280280
action_cache_is_corrupted: info.origin.guaranteed_by_action_cache()
281281
)?;
@@ -297,7 +297,7 @@ impl Uploader {
297297
file.digest,
298298
file.digest.expires(),
299299
err
300-
),
300+
).into(),
301301
quiet: true
302302
)?;
303303

app/buck2_execute_impl/src/executors/local.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -879,7 +879,8 @@ pub async fn materialize_inputs(
879879
task: false,
880880
daemon_in_memory_state_is_corrupted: true,
881881
action_cache_is_corrupted: corrupted
882-
));
882+
)
883+
.into());
883884
}
884885
Err(e) => {
885886
return Err(e.into());
@@ -914,7 +915,7 @@ async fn check_inputs(
914915
// want to propagate it.
915916
let _ignored = tag_result!(
916917
"missing_local_inputs",
917-
fs_util::symlink_metadata(&abs_path).context("Missing input"),
918+
fs_util::symlink_metadata(&abs_path).context("Missing input").map_err(|e| e.into()),
918919
quiet: true,
919920
task: false,
920921
daemon_materializer_state_is_corrupted: true

app/buck2_execute_impl/src/executors/re.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ impl ReExecutor {
252252
execution_time.as_secs(),
253253
timeout.as_secs(),
254254
)
255+
.into()
255256
);
256257

257258
if let Err(e) = res {

app/buck2_execute_impl/src/materializers/deferred.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1341,7 +1341,7 @@ impl<T: IoHandler> DeferredMaterializerCommandProcessor<T> {
13411341
// This should never happen
13421342
soft_error!(
13431343
"clean_stale_no_config",
1344-
anyhow::anyhow!("clean scheduled without being configured"),
1344+
anyhow::anyhow!("clean scheduled without being configured").into(),
13451345
quiet: true
13461346
)
13471347
.unwrap();
@@ -1519,7 +1519,7 @@ impl<T: IoHandler> DeferredMaterializerCommandProcessor<T> {
15191519
{
15201520
soft_error!(
15211521
"materializer_materialize_error",
1522-
e.context(self.log_buffer.clone()),
1522+
e.context(self.log_buffer.clone()).into(),
15231523
quiet: true
15241524
)
15251525
.unwrap();
@@ -1774,7 +1774,7 @@ impl<T: IoHandler> DeferredMaterializerCommandProcessor<T> {
17741774
.materializer_state_table()
17751775
.update_access_times(vec![&path])
17761776
{
1777-
soft_error!("has_artifact_update_time", e.context(self.log_buffer.clone()), quiet: true).unwrap();
1777+
soft_error!("has_artifact_update_time", e.context(self.log_buffer.clone()).into(), quiet: true).unwrap();
17781778
}
17791779
}
17801780
}
@@ -2155,7 +2155,7 @@ fn on_materialization(
21552155
.materializer_state_table()
21562156
.insert(path, metadata, timestamp)
21572157
{
2158-
soft_error!(error_name, e.context(log_buffer.clone()), quiet: true).unwrap();
2158+
soft_error!(error_name, e.context(log_buffer.clone()).into(), quiet: true).unwrap();
21592159
}
21602160
}
21612161

@@ -2299,7 +2299,7 @@ impl ArtifactTree {
22992299
}
23002300
Err(e) => {
23012301
// NOTE: This shouldn't normally happen?
2302-
soft_error!("cleanup_finished_vacant", e, quiet: true).unwrap();
2302+
soft_error!("cleanup_finished_vacant", e.into(), quiet: true).unwrap();
23032303
}
23042304
}
23052305
}

app/buck2_execute_impl/src/materializers/deferred/clean_stale.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,8 @@ impl CleanStaleArtifactsCommand {
285285
stats,
286286
};
287287
// quiet just because it's also returned, soft_error to log to scribe
288-
return Err(soft_error!("clean_stale_error", error.into(), quiet: true)?);
288+
return Err(soft_error!("clean_stale_error", error.into(), quiet: true)
289+
.map(|e| e.into())?);
289290
}
290291
}
291292

app/buck2_forkserver/src/client.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,8 @@ impl ForkserverClient {
102102
quiet: true,
103103
task: false,
104104
daemon_in_memory_state_is_corrupted: true,
105-
));
105+
)
106+
.into());
106107
}
107108

108109
let stream = stream::once(future::ready(buck2_forkserver_proto::RequestEvent {

app/buck2_http/src/client/builder.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ impl HttpClientBuilder {
9191
} else {
9292
soft_error!(
9393
"http_client_no_certs",
94-
anyhow::anyhow!("Using default http client with no certs"),
94+
anyhow::anyhow!("Using default http client with no certs").into(),
9595
quiet: true
9696
)?;
9797
}

app/buck2_interpreter/src/soft_error.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,14 @@
99

1010
use buck2_core::soft_error;
1111
use starlark::eval::SoftErrorHandler;
12-
12+
use starlark::ErrorKind;
1313
pub struct Buck2StarlarkSoftErrorHandler;
1414

1515
/// When starlark deprecates something, we propagate it to our `soft_error!` handler.
1616
impl SoftErrorHandler for Buck2StarlarkSoftErrorHandler {
1717
fn soft_error(&self, category: &str, error: starlark::Error) -> Result<(), starlark::Error> {
18-
soft_error!(&format!("starlark_rust_{category}"), error.into_anyhow(), quiet:true)?;
18+
soft_error!(&format!("starlark_rust_{category}"), error.into_anyhow().into(), quiet:true)
19+
.map_err(|e| starlark::Error::new(ErrorKind::Other(e.into())))?;
1920
Ok(())
2021
}
2122
}

app/buck2_node/src/attrs/spec.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ impl AttributeSpec {
128128
if name == "metadata" {
129129
soft_error!(
130130
"metadata_attribute",
131-
anyhow::anyhow!("Rules should not declare an attribute named metadata`"),
131+
anyhow::anyhow!("Rules should not declare an attribute named metadata`").into(),
132132
quiet: true
133133
)?;
134134
}

app/buck2_server/src/daemon/state.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -711,7 +711,7 @@ impl DaemonState {
711711

712712
tag_result!(
713713
"eden_not_connected",
714-
check_working_dir::check_working_dir(),
714+
check_working_dir::check_working_dir().map_err(|e| e.into()),
715715
quiet: true,
716716
daemon_in_memory_state_is_corrupted: true,
717717
task: false
@@ -765,7 +765,7 @@ impl DaemonState {
765765

766766
tag_result!(
767767
"stale_cwd",
768-
res,
768+
res.map_err(|e| e.into()),
769769
quiet: true,
770770
daemon_in_memory_state_is_corrupted: true,
771771
task: false
@@ -817,6 +817,7 @@ impl DaemonState {
817817
This will likely lead to failed or slow builds. \
818818
To remediate, run `eden redirect fixup`."
819819
)
820+
.into()
820821
)?;
821822
}
822823

app/buck2_server/src/host_info.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ pub fn get_host_info(
6565
{
6666
Ok(v) => v,
6767
Err(e) => {
68-
soft_error!("invalid_xcode_version", e)?;
68+
soft_error!("invalid_xcode_version", e.into())?;
6969
None
7070
}
7171
}

app/buck2_server_ctx/src/concurrency.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,7 @@ impl ConcurrencyHandler {
644644
active_commands,
645645
current_command.format_argv(),
646646
))
647+
.into()
647648
)?;
648649
}
649650
_ => {}

app/buck2_test/src/command.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ async fn test_targets(
476476

477477
let res = tag_result!(
478478
"executor_launch_failed",
479-
res,
479+
res.map_err(|e| e.into()),
480480
quiet: true,
481481
daemon_in_memory_state_is_corrupted: true,
482482
task: false

app/buck2_test/src/local_resource_setup.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ async fn required_providers<'v>(
9292
Ok(Some(x)) => Some(Ok(x)),
9393
Ok(None) => None,
9494
Err(e) => {
95-
let _ignore = soft_error!("missing_required_local_resource", e, quiet: true);
95+
let _ignore = soft_error!("missing_required_local_resource", e.into(), quiet: true);
9696
None
9797
}
9898
})

0 commit comments

Comments
 (0)