Merge "tp: Implement error offset in messaging"
diff --git a/include/perfetto/ext/base/string_utils.h b/include/perfetto/ext/base/string_utils.h
index 593204f..30f8b9d 100644
--- a/include/perfetto/ext/base/string_utils.h
+++ b/include/perfetto/ext/base/string_utils.h
@@ -167,6 +167,20 @@
size_t SprintfTrunc(char* dst, size_t dst_size, const char* fmt, ...)
PERFETTO_PRINTF_FORMAT(3, 4);
+// Line number starts from 1
+struct LineWithOffset {
+ base::StringView line;
+ uint32_t line_offset;
+ uint32_t line_num;
+};
+
+// For given string and offset Pfinds a line with character for
+// which offset points, what number is this line (starts from 1), and the offset
+// inside this line. returns nullopt if the offset points to
+// line break character or exceeds string length.
+base::Optional<LineWithOffset> FindLineWithOffset(base::StringView str,
+ uint32_t offset);
+
// A helper class to facilitate construction and usage of write-once stack
// strings.
// Example usage:
diff --git a/src/base/string_utils.cc b/src/base/string_utils.cc
index 7e98ecd..e33bf9f 100644
--- a/src/base/string_utils.cc
+++ b/src/base/string_utils.cc
@@ -233,5 +233,28 @@
return res;
}
+base::Optional<LineWithOffset> FindLineWithOffset(base::StringView str,
+ uint32_t offset) {
+ static constexpr char kNewLine = '\n';
+ uint32_t line_offset = 0;
+ uint32_t line_count = 1;
+ for (uint32_t i = 0; i < str.size(); ++i) {
+ if (str.at(i) == kNewLine) {
+ line_offset = i + 1;
+ line_count++;
+ continue;
+ }
+ if (i == offset) {
+ size_t end_offset = str.find(kNewLine, i);
+ if (end_offset == std::string::npos) {
+ end_offset = str.size();
+ }
+ base::StringView line = str.substr(line_offset, end_offset - line_offset);
+ return LineWithOffset{line, offset - line_offset, line_count};
+ }
+ }
+ return base::nullopt;
+}
+
} // namespace base
} // namespace perfetto
diff --git a/src/base/string_utils_unittest.cc b/src/base/string_utils_unittest.cc
index 9a885d1..35bc169 100644
--- a/src/base/string_utils_unittest.cc
+++ b/src/base/string_utils_unittest.cc
@@ -416,6 +416,72 @@
}
}
+TEST(FindLineTest, InvalidOffset1) {
+ std::string str = "abc\ndef\n\nghi";
+ uint32_t offset = 3;
+
+ auto error = FindLineWithOffset(base::StringView(str), offset);
+
+ EXPECT_FALSE(error.has_value());
+}
+
+TEST(FindLineTest, InvalidOffset2) {
+ std::string str = "abc\ndef\n\nghi";
+ uint32_t offset = 8;
+
+ auto error = FindLineWithOffset(base::StringView(str), offset);
+
+ EXPECT_FALSE(error.has_value());
+}
+
+TEST(FindLineTest, FirstCharacter) {
+ std::string str = "abc\ndef\n\nghi";
+ uint32_t offset = 0;
+
+ auto error = FindLineWithOffset(base::StringView(str), offset);
+
+ EXPECT_TRUE(error.has_value());
+ ASSERT_EQ(error.value().line_num, 1ul);
+ ASSERT_EQ(error.value().line_offset, 0ul);
+ ASSERT_EQ(error.value().line, "abc");
+}
+
+TEST(FindLineTest, StandardCheck) {
+ std::string str = "abc\ndef\n\nghi";
+ uint32_t offset = 5;
+
+ auto error = FindLineWithOffset(base::StringView(str), offset);
+
+ EXPECT_TRUE(error.has_value());
+ ASSERT_EQ(error.value().line_num, 2ul);
+ ASSERT_EQ(error.value().line_offset, 1ul);
+ ASSERT_EQ(error.value().line, "def");
+}
+
+TEST(FindLineTest, TwoBreakLines) {
+ std::string str = "abc\ndef\n\nghi";
+ uint32_t offset = 10;
+
+ auto error = FindLineWithOffset(base::StringView(str), offset);
+
+ EXPECT_TRUE(error.has_value());
+ ASSERT_EQ(error.value().line_num, 4ul);
+ ASSERT_EQ(error.value().line_offset, 1ul);
+ ASSERT_EQ(error.value().line, "ghi");
+}
+
+TEST(FindLineTest, EndsWithBreakLine) {
+ std::string str = "abc\ndef\n\nghi\n";
+ uint32_t offset = 10;
+
+ auto error = FindLineWithOffset(base::StringView(str), offset);
+
+ EXPECT_TRUE(error.has_value());
+ ASSERT_EQ(error.value().line_num, 4ul);
+ ASSERT_EQ(error.value().line_offset, 1ul);
+ ASSERT_EQ(error.value().line, "ghi");
+}
+
} // namespace
} // namespace base
} // namespace perfetto
diff --git a/src/trace_processor/iterator_impl.h b/src/trace_processor/iterator_impl.h
index 68068e6..2372bab 100644
--- a/src/trace_processor/iterator_impl.h
+++ b/src/trace_processor/iterator_impl.h
@@ -85,7 +85,9 @@
int ret = sqlite3_step(*stmt_);
if (PERFETTO_UNLIKELY(ret != SQLITE_ROW && ret != SQLITE_DONE)) {
- status_ = base::ErrStatus("%s (errcode %d)", sqlite3_errmsg(db_), ret);
+ status_ = base::ErrStatus(
+ "%s",
+ sqlite_utils::FormatErrorMessage(stmt_.get(), db_, ret).c_message());
stmt_.reset();
return false;
}
diff --git a/src/trace_processor/rpc/query_result_serializer_unittest.cc b/src/trace_processor/rpc/query_result_serializer_unittest.cc
index 85c882d..6344edc 100644
--- a/src/trace_processor/rpc/query_result_serializer_unittest.cc
+++ b/src/trace_processor/rpc/query_result_serializer_unittest.cc
@@ -408,7 +408,7 @@
TestDeserializer deser;
deser.SerializeAndDeserialize(&ser);
EXPECT_EQ(deser.cells.size(), 0u);
- EXPECT_EQ(deser.error, "incomplete input (errcode: 1)");
+ EXPECT_EQ(deser.error, "Error: incomplete input (errcode: 1)");
EXPECT_TRUE(deser.eof_reached);
}
diff --git a/src/trace_processor/sqlite/create_function.cc b/src/trace_processor/sqlite/create_function.cc
index cf5aa54..1a840ef 100644
--- a/src/trace_processor/sqlite/create_function.cc
+++ b/src/trace_processor/sqlite/create_function.cc
@@ -257,9 +257,9 @@
if (ret != SQLITE_OK) {
return base::ErrStatus(
"CREATE_FUNCTION[prototype=%s]: SQLite error when preparing "
- "statement "
- "%s",
- prototype_str.ToStdString().c_str(), sqlite3_errmsg(ctx->db));
+ "statement %s",
+ prototype_str.ToStdString().c_str(),
+ sqlite_utils::FormatErrorMessage(stmt_raw, ctx->db, ret).c_message());
}
stmt.reset(stmt_raw);
diff --git a/src/trace_processor/sqlite/create_view_function.cc b/src/trace_processor/sqlite/create_view_function.cc
index 3abeef4..b64cd4b 100644
--- a/src/trace_processor/sqlite/create_view_function.cc
+++ b/src/trace_processor/sqlite/create_view_function.cc
@@ -126,9 +126,9 @@
if (ret != SQLITE_OK) {
return base::ErrStatus(
"%s: Failed to prepare SQL statement for function. "
- "Check the SQL defintion this function for syntax errors. "
- "(SQLite error: %s).",
- prototype_.function_name.c_str(), sqlite3_errmsg(db_));
+ "Check the SQL defintion this function for syntax errors.\n%s",
+ prototype_.function_name.c_str(),
+ sqlite_utils::FormatErrorMessage(raw_stmt, db_, ret).c_message());
}
// Verify that every argument name in the function appears in the
@@ -349,7 +349,8 @@
if (ret != SQLITE_ROW && ret != SQLITE_DONE) {
table_->SetErrorMessage(sqlite3_mprintf(
"%s: SQLite error while stepping statement: %s",
- table_->prototype_.function_name.c_str(), sqlite3_errmsg(table_->db_)));
+ table_->prototype_.function_name.c_str(),
+ sqlite_utils::FormatErrorMessage(stmt_, table_->db_, ret).c_message()));
return ret;
}
return SQLITE_OK;
diff --git a/src/trace_processor/sqlite/span_join_operator_table.cc b/src/trace_processor/sqlite/span_join_operator_table.cc
index 0f75af7..4824da1f 100644
--- a/src/trace_processor/sqlite/span_join_operator_table.cc
+++ b/src/trace_processor/sqlite/span_join_operator_table.cc
@@ -748,7 +748,9 @@
cursor_eof_ = res != SQLITE_OK;
if (res != SQLITE_OK)
- return util::ErrStatus("%s", sqlite3_errmsg(db_));
+ return util::ErrStatus(
+ "%s",
+ sqlite_utils::FormatErrorMessage(stmt_.get(), db_, res).c_message());
RETURN_IF_ERROR(CursorNext());
diff --git a/src/trace_processor/sqlite/sqlite_utils.h b/src/trace_processor/sqlite/sqlite_utils.h
index 90a98bc..b2adf8a 100644
--- a/src/trace_processor/sqlite/sqlite_utils.h
+++ b/src/trace_processor/sqlite/sqlite_utils.h
@@ -19,10 +19,14 @@
#include <math.h>
#include <sqlite3.h>
-#include <string>
+#include <cstddef>
+#include <cstring>
+#include "perfetto/base/logging.h"
#include "perfetto/base/status.h"
+#include "perfetto/ext/base/optional.h"
#include "perfetto/ext/base/string_utils.h"
+#include "perfetto/ext/base/string_view.h"
#include "perfetto/trace_processor/basic_types.h"
#include "src/trace_processor/sqlite/scoped_db.h"
#include "src/trace_processor/sqlite/sqlite_table.h"
@@ -133,6 +137,44 @@
}
}
+inline ScopedSqliteString ExpandedSqlForStmt(sqlite3_stmt* stmt) {
+ return ScopedSqliteString(sqlite3_expanded_sql(stmt));
+}
+
+inline base::Status FormatErrorMessage(base::StringView sql,
+ sqlite3* db,
+ int error_code) {
+ uint32_t offset = static_cast<uint32_t>(sqlite3_error_offset(db));
+
+ auto error_opt = FindLineWithOffset(sql, offset);
+
+ if (!error_opt.has_value()) {
+ return base::ErrStatus("Error: %s (errcode: %d)", sqlite3_errmsg(db),
+ error_code);
+ }
+
+ auto error = error_opt.value();
+
+ return base::ErrStatus(
+ "Error in line:%u, col: %u.\n"
+ "%s\n"
+ "%s^\n"
+ "%s (errcode: %d)",
+ error.line_num, error.line_offset + 1, error.line.ToStdString().c_str(),
+ std::string(error.line_offset, ' ').c_str(), sqlite3_errmsg(db),
+ error_code);
+}
+
+inline base::Status FormatErrorMessage(sqlite3_stmt* stmt,
+ sqlite3* db,
+ int error_code) {
+ PERFETTO_CHECK(stmt);
+ auto sql = ExpandedSqlForStmt(stmt);
+ PERFETTO_CHECK(sql);
+ base::Status error_msg_status = FormatErrorMessage(sql.get(), db, error_code);
+ return error_msg_status;
+}
+
inline base::Status PrepareStmt(sqlite3* db,
const char* sql,
ScopedStmt* stmt,
@@ -141,14 +183,10 @@
int err = sqlite3_prepare_v2(db, sql, -1, &raw_stmt, tail);
stmt->reset(raw_stmt);
if (err != SQLITE_OK)
- return base::ErrStatus("%s (errcode: %d)", sqlite3_errmsg(db), err);
+ return base::ErrStatus("%s", FormatErrorMessage(sql, db, err).c_message());
return base::OkStatus();
}
-inline ScopedSqliteString ExpandedSqlForStmt(sqlite3_stmt* stmt) {
- return ScopedSqliteString(sqlite3_expanded_sql(stmt));
-}
-
inline bool IsStmtDone(sqlite3_stmt* stmt) {
return !sqlite3_stmt_busy(stmt);
}
@@ -163,8 +201,8 @@
for (err = sqlite3_step(stmt); err == SQLITE_ROW; err = sqlite3_step(stmt)) {
}
if (err != SQLITE_DONE) {
- return base::ErrStatus("%s (errcode: %d)",
- sqlite3_errmsg(sqlite3_db_handle(stmt)), err);
+ auto db = sqlite3_db_handle(stmt);
+ return base::ErrStatus("%s", FormatErrorMessage(stmt, db, err).c_message());
}
return base::OkStatus();
}
diff --git a/src/trace_processor/sqlite/sqlite_utils_unittest.cc b/src/trace_processor/sqlite/sqlite_utils_unittest.cc
index 1d1de13..5d85383 100644
--- a/src/trace_processor/sqlite/sqlite_utils_unittest.cc
+++ b/src/trace_processor/sqlite/sqlite_utils_unittest.cc
@@ -16,6 +16,7 @@
#include "src/trace_processor/sqlite/sqlite_utils.h"
+#include "gtest/gtest.h"
#include "test/gtest_and_gmock.h"
namespace perfetto {
diff --git a/src/trace_processor/trace_processor_impl.cc b/src/trace_processor/trace_processor_impl.cc
index caa8d21..ad6fa90 100644
--- a/src/trace_processor/trace_processor_impl.cc
+++ b/src/trace_processor/trace_processor_impl.cc
@@ -977,8 +977,11 @@
// Now step once into |cur_stmt| so that when we prepare the next statment
// we will have executed any dependent bytecode in this one.
int err = sqlite3_step(*cur_stmt);
- if (err != SQLITE_ROW && err != SQLITE_DONE)
- return base::ErrStatus("%s (errcode: %d)", sqlite3_errmsg(db), err);
+ if (err != SQLITE_ROW && err != SQLITE_DONE) {
+ return base::ErrStatus(
+ "%s", sqlite_utils::FormatErrorMessage(prev_stmt.get(), db, err)
+ .c_message());
+ }
}
// Increment the neecessary counts for the statement.
