From 0ac40cd4cd6b16ecd99fbab2f51269cfdf59e78e Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Thu, 19 Oct 2023 13:25:40 -0400 Subject: [PATCH] src.ctf.fs: improve `metadata-info` query error message with non-existent metadata file A colleague pointed out that the message babeltrace2 outputs when pointing the `metadata-info` query to a directory without a metadata file (or a non-existent directory) is not clear as it could be. ERROR: [Babeltrace CLI] (/home/smarchi/src/babeltrace/src/cli/babeltrace2.c:651) Failed to query component class: unknown error: plugin-name="ctf", comp-cls-name="fs", comp-cls-type=SOURCE object="metadata-info" CAUSED BY [libbabeltrace2] (/home/smarchi/src/babeltrace/src/lib/graph/query-executor.c:234) Component class's "query" method failed: query-exec-addr=0x60b000000880, cc-addr=0x60f000000130, cc-type=SOURCE, cc-name="fs", cc-partial-descr="Read CTF traces from the file sy", cc-is-frozen=0, cc-so-handle-addr=0x607000000020, cc-so-handle-path="/home/smarchi/build/babeltrace/src/plugins/ctf/babeltrace-plugin-ctf.la", object="metadata-info", params-addr=0x606000000500, params-type=MAP, params-element-count=1, log-level=WARNING CAUSED BY ['source.ctf.fs'] (/home/smarchi/src/babeltrace/src/plugins/ctf/fs-src/query.cpp:92) Cannot open trace metadata: path="yoyoyo". In the ctf_fs_metadata_open_file function, we could have access to the errno that explains the failure, but currently don't use it. Also, it would be good to tell which file we tried to open exactly. Make ctf_fs_metadata_open_file log an error and append an error cause. The new error stack is the same as the above, with this new cause at the end: CAUSED BY ['source.ctf.fs'] (/home/smarchi/src/babeltrace/src/plugins/ctf/fs-src/metadata.cpp:41) Failed to open metadata file: No such file or directory: path="yoyoyo/metadata" Add a test that validates that this error cause is present in the error message. Change-Id: I111848cc72e6f3b2cdfb251914eb337ea02a19f4 Reviewed-on: https://review.lttng.org/c/babeltrace/+/11079 Tested-by: jenkins Reviewed-by: Michael Jeanson Reviewed-by: Philippe Proulx CI-Build: Simon Marchi --- src/plugins/ctf/fs-src/metadata.cpp | 9 +++++- src/plugins/ctf/fs-src/metadata.hpp | 3 +- src/plugins/ctf/fs-src/query.cpp | 2 +- .../src.ctf.fs/query/test_query_metadata_info | 31 ++++++++++++++++++- 4 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/plugins/ctf/fs-src/metadata.cpp b/src/plugins/ctf/fs-src/metadata.cpp index f428c5e0..0681b6f2 100644 --- a/src/plugins/ctf/fs-src/metadata.cpp +++ b/src/plugins/ctf/fs-src/metadata.cpp @@ -24,7 +24,8 @@ #include "metadata.hpp" #include "../common/metadata/decoder.hpp" -FILE *ctf_fs_metadata_open_file(const char *trace_path) +FILE *ctf_fs_metadata_open_file(const char *trace_path, bt_logging_level log_level, + bt_self_component_class *comp_class) { GString *metadata_path; FILE *fp = NULL; @@ -36,7 +37,13 @@ FILE *ctf_fs_metadata_open_file(const char *trace_path) g_string_append(metadata_path, G_DIR_SEPARATOR_S CTF_FS_METADATA_FILENAME); fp = fopen(metadata_path->str, "rb"); + if (!fp) { + BT_COMP_CLASS_LOGE_APPEND_CAUSE_ERRNO(comp_class, "Failed to open metadata file", + ": path=\"%s\"", metadata_path->str); + } + g_string_free(metadata_path, TRUE); + end: return fp; } diff --git a/src/plugins/ctf/fs-src/metadata.hpp b/src/plugins/ctf/fs-src/metadata.hpp index fd72e157..e0298393 100644 --- a/src/plugins/ctf/fs-src/metadata.hpp +++ b/src/plugins/ctf/fs-src/metadata.hpp @@ -32,7 +32,8 @@ void ctf_fs_metadata_fini(struct ctf_fs_metadata *metadata); int ctf_fs_metadata_set_trace_class(bt_self_component *self_comp, struct ctf_fs_trace *ctf_fs_trace, struct ctf_fs_metadata_config *config); -FILE *ctf_fs_metadata_open_file(const char *trace_path); +FILE *ctf_fs_metadata_open_file(const char *trace_path, bt_logging_level log_level, + bt_self_component_class *comp_class); bool ctf_metadata_is_packetized(FILE *fp, int *byte_order); diff --git a/src/plugins/ctf/fs-src/query.cpp b/src/plugins/ctf/fs-src/query.cpp index af11bef5..2f51f709 100644 --- a/src/plugins/ctf/fs-src/query.cpp +++ b/src/plugins/ctf/fs-src/query.cpp @@ -87,7 +87,7 @@ metadata_info_query(bt_self_component_class_source *self_comp_class_src, const b path = bt_value_string_get(path_value); BT_ASSERT(path); - metadata_fp = ctf_fs_metadata_open_file(path); + metadata_fp = ctf_fs_metadata_open_file(path, log_level, self_comp_class); if (!metadata_fp) { BT_COMP_CLASS_LOGE_APPEND_CAUSE(self_comp_class, "Cannot open trace metadata: path=\"%s\".", path); diff --git a/tests/plugins/src.ctf.fs/query/test_query_metadata_info b/tests/plugins/src.ctf.fs/query/test_query_metadata_info index 6db90091..2efa9e9a 100755 --- a/tests/plugins/src.ctf.fs/query/test_query_metadata_info +++ b/tests/plugins/src.ctf.fs/query/test_query_metadata_info @@ -50,5 +50,34 @@ test_query_metadata_info() { rm -f "$temp_stdout_output_file" "$temp_stderr_output_file" } -plan_tests 1 +test_non_existent_trace_dir() { + local empty_dir + local stdout_file + local stderr_file + local query + + empty_dir=$(mktemp -d) + stdout_file="$(mktemp -t actual_stdout.XXXXXX)" + stderr_file="$(mktemp -t actual_stderr.XXXXXX)" + query=("query" "src.ctf.fs" "metadata-info" "--params" "path=\"$empty_dir\"") + + bt_cli "$stdout_file" "$stderr_file" \ + "${query[@]}" + isnt $? 0 "non existent trace dir: babeltrace exits with an error" + + bt_diff "/dev/null" "${stdout_file}" + ok $? "non existent trace dir: babeltrace produces the expected stdout" + + grep --silent "^CAUSED BY " "${stderr_file}" + ok $? "non existent trace dir: babeltrace produces an error stack" + + grep --silent "Failed to open metadata file: No such file or directory: path=\".*metadata\"" \ + "${stderr_file}" + ok $? "non existent trace dir: babeltrace produces the expected error message" + + rm -f "${stdout_file}" "${stderr_file}" +} + +plan_tests 5 test_query_metadata_info succeed1 +test_non_existent_trace_dir -- 2.34.1