Skip to content

Commit

Permalink
Add test for buck2_error->Starlark conversion bug
Browse files Browse the repository at this point in the history
Summary: When a `buck2_error::Error` that has a `TypedContext` within its error stack, the conversion to `Starlark::Error` will cause the root error to be dropped which isn't the right behavior. This diff adds the test showing the failing behavior, next diff will be the fix.

Reviewed By: JakobDegen

Differential Revision: D68570401

fbshipit-source-id: 2617eb9f4322cc949e39a254ed29d1fb3fb1d41e
  • Loading branch information
Will-MingLun-Li authored and facebook-github-bot committed Jan 24, 2025
1 parent 51314c5 commit 0f9c49c
Showing 1 changed file with 36 additions and 1 deletion.
37 changes: 36 additions & 1 deletion app/buck2_error/src/starlark_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,14 +221,17 @@ impl std::error::Error for StarlarkErrorWrapper {}
#[cfg(test)]
mod tests {
use std::error::Request;
use std::sync::Arc;

use allocative::Allocative;

use crate::any::ProvidableMetadata;
use crate::buck2_error;
use crate::context_value::StarlarkContext;
use crate::source_location::SourceLocation;
use crate::starlark_error::error_with_starlark_context;

#[derive(Debug, derive_more::Display)]
#[derive(Debug, Allocative, derive_more::Display)]
struct FullMetadataError;

impl From<FullMetadataError> for crate::Error {
Expand All @@ -254,6 +257,12 @@ mod tests {
}
}

impl crate::TypedContext for FullMetadataError {
fn eq(&self, _other: &dyn crate::TypedContext) -> bool {
true
}
}

#[test]
fn test_roundtrip_starlark() {
// Tests buck2_error->starlark->buck2_error
Expand Down Expand Up @@ -330,4 +339,30 @@ mod tests {
assert!(base_replaced.to_string().contains(starlark_call_stack));
assert!(base_replaced.to_string().contains(starlark_error));
}

#[test]
fn test_conversion_with_typed_context() {
let base_error = "Some base error";
let e = buck2_error!(crate::ErrorTag::StarlarkError, "{}", base_error);
let e = e.compute_context(
|_: Arc<FullMetadataError>| FullMetadataError,
|| FullMetadataError,
);

let starlark_call_stack = "Some call stack";
let starlark_error_msg = "Starlark error message";
let starlark_context = StarlarkContext {
call_stack: starlark_call_stack.to_owned(),
error_msg: starlark_error_msg.to_owned(),
span: None,
};

let starlark_err = error_with_starlark_context(&e, starlark_context);
let starlark_err_string = format!("{:#}", starlark_err);

assert!(starlark_err_string.contains(starlark_call_stack));
assert!(starlark_err_string.contains(starlark_error_msg));
// FIXME(minglunli): Root error shouldn't be lost
assert!(!starlark_err_string.contains(base_error));
}
}

0 comments on commit 0f9c49c

Please sign in to comment.