Skip to content

fix: add backtrace for assert_*_or_internal_err helpers#18910

Merged
rluvaton merged 10 commits into
apache:mainfrom
rluvaton:move-to-use-macro-for-backtrace
Jun 10, 2026
Merged

fix: add backtrace for assert_*_or_internal_err helpers#18910
rluvaton merged 10 commits into
apache:mainfrom
rluvaton:move-to-use-macro-for-backtrace

Conversation

@rluvaton

@rluvaton rluvaton commented Nov 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Which issue does this PR close?

N/A

Rationale for this change

No backtrace is added when using the assert macros, so fixing that

What changes are included in this PR?

used internal_datafusion_err macro in the assert_* helpers

Are these changes tested?

yes

Are there any user-facing changes?

now have backtrace when feature enabled

@github-actions github-actions Bot added logical-expr Logical plan and expressions common Related to common crate labels Nov 24, 2025
@rluvaton rluvaton marked this pull request as draft November 24, 2025 14:38
@github-actions github-actions Bot added sql SQL Planner physical-expr Changes to the physical-expr crates optimizer Optimizer rules catalog Related to the catalog crate proto Related to proto crate functions Changes to functions implementation datasource Changes to the datasource crate physical-plan Changes to the physical-plan crate spark labels Nov 24, 2025
@github-actions

github-actions Bot commented Jan 26, 2026

Copy link
Copy Markdown

Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.

@github-actions github-actions Bot added the Stale PR has not had any activity for some time label Jan 26, 2026
@github-actions github-actions Bot closed this Feb 3, 2026
@rluvaton rluvaton reopened this Jun 10, 2026
@github-actions github-actions Bot removed sql SQL Planner logical-expr Logical plan and expressions physical-expr Changes to the physical-expr crates optimizer Optimizer rules catalog Related to the catalog crate proto Related to proto crate functions Changes to functions implementation datasource Changes to the datasource crate physical-plan Changes to the physical-plan crate labels Jun 10, 2026
@github-actions github-actions Bot removed the spark label Jun 10, 2026
@rluvaton rluvaton removed the Stale PR has not had any activity for some time label Jun 10, 2026
@rluvaton rluvaton marked this pull request as ready for review June 10, 2026 11:01
@rluvaton rluvaton changed the title chore: use internal_err macro in assert_eq_or_internal_err for backtrace chore: use internal_err macro in assert_*_or_internal_err helpers for backtrace Jun 10, 2026
@rluvaton rluvaton changed the title chore: use internal_err macro in assert_*_or_internal_err helpers for backtrace fix: add backtrace for assert_*_or_internal_err helpers Jun 10, 2026
@github-actions github-actions Bot added the auto detected api change Auto detected API change label Jun 10, 2026
@github-actions github-actions Bot removed the auto detected api change Auto detected API change label Jun 10, 2026
@rluvaton rluvaton mentioned this pull request Jun 10, 2026
Open
@Weijun-H

Weijun-H commented Jun 10, 2026

Copy link
Copy Markdown
Member

This PR breaks the backtrace-enabled tests for Internal errors.

I can reproduce this locally:

RUST_BACKTRACE=1 cargo test --profile ci --exclude datafusion-examples --exclude ffi_example_table_provider --exclude datafusion-cli --workspace --lib --tests --bins --features serde,avro,json,backtrace,integration-tests,parquet_encryption,substrait test_enabled_backtrace_for_assert_or_internal_err_without_args -- --nocapture

This fails in assert_error_have_message_and_backtrace, and the broader backtrace test filter also fails for all 7 newly added test_enabled_backtrace_for_* cases.

rluvaton
rluvaton commented Jun 10, 2026
View reviewed changes
Comment thread datafusion/common/src/error.rs
Comment on lines 1207 to 1315
@@ -1223,14 +1222,8 @@ mod test {
ok_result()
}

let err = check().unwrap_err();
assert_snapshot!(
err.to_string(),
@r"
Internal error: Assertion failed: 1 == 2 (left: 1, right: 2): expected equality.
This issue was likely caused by a bug in DataFusion's code. Please help us to resolve this by filing a bug report in our issue tracker: https://github.com/apache/datafusion/issues
"
);
let err = check().unwrap_err().strip_backtrace();
assert!(err.starts_with("Internal error: Assertion failed: 1 == 2 (left: 1, right: 2): expected equality"));
}

#[test]
@@ -1246,14 +1239,8 @@ mod test {
ok_result()
}

let err = check().unwrap_err();
assert_snapshot!(
err.to_string(),
@r"
Internal error: Assertion failed: 3 != 3 (left: 3, right: 3): values must differ.
This issue was likely caused by a bug in DataFusion's code. Please help us to resolve this by filing a bug report in our issue tracker: https://github.com/apache/datafusion/issues
"
);
let err = check().unwrap_err().strip_backtrace();
assert!(err.starts_with("Internal error: Assertion failed: 3 != 3 (left: 3, right: 3): values must differ"));
}

#[test]
@@ -1270,14 +1257,8 @@ mod test {
ok_result()
}

let err = check().unwrap_err();
assert_snapshot!(
err.to_string(),
@r"
Internal error: Assertion failed: false.
This issue was likely caused by a bug in DataFusion's code. Please help us to resolve this by filing a bug report in our issue tracker: https://github.com/apache/datafusion/issues
"
);
let err = check().unwrap_err().strip_backtrace();
assert!(err.starts_with("Internal error: Assertion failed: false"));
}

#[test]
@@ -1287,13 +1268,9 @@ mod test {
ok_result()
}

let err = check().unwrap_err();
assert_snapshot!(
err.to_string(),
@r"
Internal error: Assertion failed: false: custom message.
This issue was likely caused by a bug in DataFusion's code. Please help us to resolve this by filing a bug report in our issue tracker: https://github.com/apache/datafusion/issues
"
let err = check().unwrap_err().strip_backtrace();
assert!(
err.starts_with("Internal error: Assertion failed: false: custom message")
);
}

@@ -1304,14 +1281,8 @@ mod test {
ok_result()
}

let err = check().unwrap_err();
assert_snapshot!(
err.to_string(),
@r"
Internal error: Assertion failed: false: custom 42.
This issue was likely caused by a bug in DataFusion's code. Please help us to resolve this by filing a bug report in our issue tracker: https://github.com/apache/datafusion/issues
"
);
let err = check().unwrap_err().strip_backtrace();
assert!(err.starts_with("Internal error: Assertion failed: false: custom 42"));
}

@rluvaton rluvaton Jun 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the code to use starts_with like done in the original pr that add the backtrace:

@rluvaton

rluvaton commented Jun 10, 2026

Copy link
Copy Markdown
Member Author

@Weijun-H fixed

Weijun-H
Weijun-H approved these changes Jun 10, 2026
View reviewed changes

@Weijun-H Weijun-H left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Wait CI pass

@rluvaton rluvaton added this pull request to the merge queue Jun 10, 2026
Merged via the queue into apache:main with commit 3f52deb Jun 10, 2026
57 of 58 checks passed
@rluvaton rluvaton deleted the move-to-use-macro-for-backtrace branch June 10, 2026 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Weijun-H Weijun-H Weijun-H approved these changes

Assignees

No one assigned

Labels

common Related to common crate

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rluvaton @Weijun-H