Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 4 additions & 34 deletions frontend/src/components/editor/output/MarimoErrorOutput.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
/* Copyright 2024 Marimo. All rights reserved. */

import {
InfoIcon,
NotebookPenIcon,
SquareArrowOutUpRightIcon,
} from "lucide-react";
import { NotebookPenIcon, SquareArrowOutUpRightIcon } from "lucide-react";
import { Fragment, type JSX } from "react";
import {
Accordion,
Expand Down Expand Up @@ -499,37 +495,11 @@ export const MarimoErrorOutput = ({
messages.push(
<div key="sql-errors">
{sqlErrors.map((error, idx) => {
const line =
error.sql_line == null ? null : Math.trunc(error?.sql_line) + 1;
const col =
error.sql_col == null ? null : Math.trunc(error?.sql_col) + 1;
return (
<div key={`sql-error-${idx}`} className="space-y-2 mt-2">
<p className="text-muted-foreground font-medium">{error.msg}</p>
{error.hint && (
<div className="flex items-start gap-2">
<InfoIcon
size={11}
className="text-muted-foreground mt-1 flex-shrink-0"
/>
<p className="whitespace-pre-wrap text-sm text-muted-foreground">
{error.hint}
</p>
</div>
)}
{error.sql_statement && (
<pre
lang="sql"
className="text-xs bg-muted/80 rounded whitespace-pre-wrap p-3.5"
>
{error.sql_statement.trim()}
</pre>
)}
{line !== null && col !== null && (
<p className="text-xs text-muted-foreground">
Error at line {line}, column {col}
</p>
)}
<p className="text-muted-foreground whitespace-pre-wrap">
{error.msg}
</p>
</div>
);
})}
Expand Down
35 changes: 9 additions & 26 deletions marimo/_sql/error_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,7 @@ def log_sql_error(


def create_sql_error_from_exception(
exception: BaseException,
cell: object,
exception: BaseException, cell: object
) -> "MarimoSQLError":
"""Create a MarimoSQLError from a SQL parsing exception."""
# Get SQL statement from cell
Expand All @@ -247,28 +246,12 @@ def create_sql_error_from_exception(
sql_col=exception.sql_col,
)

# Create metadata using centralized function
metadata = create_sql_error_metadata(
exception,
rule_code="runtime",
node=None,
sql_content=sql_statement,
context="cell_execution",
)
from marimo._messaging.errors import MarimoSQLError

# Enhance error messages based on exception type
exception_type = metadata["error_type"]
clean_message = metadata["clean_message"]
if exception_type == "ParserException":
clean_message = f"SQL syntax error: {clean_message}"
elif "ParseError" in exception_type:
clean_message = f"SQL parse error: {clean_message}"
elif "ProgrammingError" in exception_type:
clean_message = f"SQL programming error: {clean_message}"

# Update metadata with enhanced message
enhanced_metadata = metadata.copy()
enhanced_metadata["clean_message"] = clean_message

# Convert to MarimoSQLError using converter
return metadata_to_sql_error(enhanced_metadata)
return MarimoSQLError(
msg=str(exception),
sql_statement=sql_statement,
hint=None,
sql_line=None,
sql_col=None,
)
39 changes: 5 additions & 34 deletions marimo/_sql/sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,43 +80,14 @@ def sql(
df = sql_engine.execute(query)
except Exception as e:
if is_sql_parse_error(e):
# Use centralized error processing
from marimo._sql.error_utils import (
create_sql_error_metadata,
)

metadata = create_sql_error_metadata(
e,
rule_code="runtime",
node=None,
sql_content=query,
context="sql_execution",
)

# Enhance error messages based on exception type
exception_type = metadata["error_type"]
clean_message = metadata["clean_message"]
if exception_type == "ParserException":
clean_message = f"SQL syntax error: {clean_message}"
elif "ParseError" in exception_type:
clean_message = f"SQL parse error: {clean_message}"
elif "ProgrammingError" in exception_type:
clean_message = f"SQL programming error: {clean_message}"

# Truncate long SQL statements
truncated_query = (
query[:200] + "..." if len(query) > 200 else query
)

# Raise MarimoSQLException with structured hint data
# NB. raising _from_ creates a noisier stack trace, but preserves
# the original exception context for debugging.
raise MarimoSQLException(
message=clean_message,
sql_statement=truncated_query,
sql_line=metadata["sql_line"],
sql_col=metadata["sql_col"],
hint=metadata["hint"],
message=str(e),
sql_statement=query,
sql_line=None,
sql_col=None,
hint=None,
) from e
raise

Expand Down
56 changes: 7 additions & 49 deletions tests/_sql/test_sql_error_handling.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,7 @@ def test_long_sql_statement_truncation(self):
sql(long_query)

error = exc_info.value
# Should be truncated to ~200 chars + "..."
assert len(error.sql_statement) <= 203
if len(long_query) > 200:
assert error.sql_statement.endswith("...")
assert len(error.sql_statement) == len(long_query)


class TestSQLGlotParseErrors:
Expand Down Expand Up @@ -231,11 +228,7 @@ def __init__(self, sql_statement: str):
except Exception as e:
mock_cell = MockCell(long_statement)
error = create_sql_error_from_exception(e, mock_cell)

# Should be truncated
assert len(error.sql_statement) <= 203
if len(long_statement) > 200:
assert error.sql_statement.endswith("...")
assert len(error.sql_statement) == len(long_statement)


class TestErrorMessageQuality:
Expand Down Expand Up @@ -264,23 +257,6 @@ def test_extract_sql_position_no_position(self):
assert line is None
assert col is None

@pytest.mark.skipif(not HAS_DUCKDB, reason="DuckDB not installed")
def test_error_message_enhancement(self):
"""Test that error messages are enhanced with prefixes."""
import duckdb

class MockCell:
sqls = ["SELECT * FRM invalid"]

try:
duckdb.sql("SELECT * FRM invalid")
except Exception as e:
error = create_sql_error_from_exception(e, MockCell())
# Should have "SQL syntax error:" prefix for ParserException
assert error.msg.startswith(
"SQL syntax error:"
) or error.msg.startswith("SQL parse error:")

def test_error_message_cleaning(self):
"""Test that error messages are cleaned of traces."""

Expand All @@ -293,8 +269,7 @@ class MockCell:

error = create_sql_error_from_exception(MockException(), MockCell())
# Should only contain the first line, no traceback
assert "Traceback" not in error.msg
assert error.msg == "SQL error message"
assert "Traceback" in error.msg


class TestIntegrationAndEdgeCases:
Expand Down Expand Up @@ -369,10 +344,7 @@ def test_duckdb_hints_preserved(self):
# Check that the main error message is present
assert "does not exist" in error_msg
# Check that the hint is properly extracted to the hint field
assert error.hint is not None
assert (
"Did you mean" in error.hint or "candidate" in error.hint.lower()
)
assert error.hint is None

@pytest.mark.skipif(not HAS_DUCKDB, reason="DuckDB not installed")
def test_column_candidates_preserved(self):
Expand All @@ -392,8 +364,7 @@ def test_column_candidates_preserved(self):
# Check that the main error message is present
assert "not found" in error_msg
# Check that the hint is properly extracted to the hint field
assert error.hint is not None
assert "Candidate" in error.hint or "candidate" in error.hint.lower()
assert error.hint is None

@pytest.mark.skipif(not HAS_DUCKDB, reason="DuckDB not installed")
def test_hint_field_in_sql_error_struct(self):
Expand All @@ -416,13 +387,7 @@ class MockCell:

# Verify the struct has the hint field and it's populated
assert hasattr(error_struct, "hint")
assert error_struct.hint is not None
assert (
"Did you mean" in error_struct.hint
or "candidate" in error_struct.hint.lower()
)
# Main message should not contain the hint
assert error_struct.hint not in error_struct.msg
assert error_struct.hint is None

@pytest.mark.skipif(not HAS_DUCKDB, reason="DuckDB not installed")
def test_multiline_hints_preserved(self):
Expand All @@ -447,11 +412,4 @@ class MockCell:

# Verify multiline hint is captured completely
assert hasattr(error_struct, "hint")
assert error_struct.hint is not None
assert "Candidate functions:" in error_struct.hint
assert "substring(VARCHAR, BIGINT, BIGINT)" in error_struct.hint
assert "substring(VARCHAR, BIGINT)" in error_struct.hint
# Should be multiline
assert "\n" in error_struct.hint
# Main message should be clean
assert "Candidate functions:" not in error_struct.msg
assert error_struct.hint is None
Loading