Skip to content

Commit 8fd0053

Browse files
committed
just return error message, and display it
1 parent 3388675 commit 8fd0053

File tree

4 files changed

+25
-143
lines changed

4 files changed

+25
-143
lines changed

frontend/src/components/editor/output/MarimoErrorOutput.tsx

Lines changed: 4 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
/* Copyright 2024 Marimo. All rights reserved. */
22

3-
import {
4-
InfoIcon,
5-
NotebookPenIcon,
6-
SquareArrowOutUpRightIcon,
7-
} from "lucide-react";
3+
import { NotebookPenIcon, SquareArrowOutUpRightIcon } from "lucide-react";
84
import { Fragment, type JSX } from "react";
95
import {
106
Accordion,
@@ -499,37 +495,11 @@ export const MarimoErrorOutput = ({
499495
messages.push(
500496
<div key="sql-errors">
501497
{sqlErrors.map((error, idx) => {
502-
const line =
503-
error.sql_line == null ? null : Math.trunc(error?.sql_line) + 1;
504-
const col =
505-
error.sql_col == null ? null : Math.trunc(error?.sql_col) + 1;
506498
return (
507499
<div key={`sql-error-${idx}`} className="space-y-2 mt-2">
508-
<p className="text-muted-foreground font-medium">{error.msg}</p>
509-
{error.hint && (
510-
<div className="flex items-start gap-2">
511-
<InfoIcon
512-
size={11}
513-
className="text-muted-foreground mt-1 flex-shrink-0"
514-
/>
515-
<p className="whitespace-pre-wrap text-sm text-muted-foreground">
516-
{error.hint}
517-
</p>
518-
</div>
519-
)}
520-
{error.sql_statement && (
521-
<pre
522-
lang="sql"
523-
className="text-xs bg-muted/80 rounded whitespace-pre-wrap p-3.5"
524-
>
525-
{error.sql_statement.trim()}
526-
</pre>
527-
)}
528-
{line !== null && col !== null && (
529-
<p className="text-xs text-muted-foreground">
530-
Error at line {line}, column {col}
531-
</p>
532-
)}
500+
<p className="text-muted-foreground whitespace-pre-wrap">
501+
{error.msg}
502+
</p>
533503
</div>
534504
);
535505
})}

marimo/_sql/error_utils.py

Lines changed: 9 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,7 @@ def log_sql_error(
225225

226226

227227
def create_sql_error_from_exception(
228-
exception: BaseException,
229-
cell: object,
228+
exception: BaseException, cell: object
230229
) -> "MarimoSQLError":
231230
"""Create a MarimoSQLError from a SQL parsing exception."""
232231
# Get SQL statement from cell
@@ -247,28 +246,12 @@ def create_sql_error_from_exception(
247246
sql_col=exception.sql_col,
248247
)
249248

250-
# Create metadata using centralized function
251-
metadata = create_sql_error_metadata(
252-
exception,
253-
rule_code="runtime",
254-
node=None,
255-
sql_content=sql_statement,
256-
context="cell_execution",
257-
)
249+
from marimo._messaging.errors import MarimoSQLError
258250

259-
# Enhance error messages based on exception type
260-
exception_type = metadata["error_type"]
261-
clean_message = metadata["clean_message"]
262-
if exception_type == "ParserException":
263-
clean_message = f"SQL syntax error: {clean_message}"
264-
elif "ParseError" in exception_type:
265-
clean_message = f"SQL parse error: {clean_message}"
266-
elif "ProgrammingError" in exception_type:
267-
clean_message = f"SQL programming error: {clean_message}"
268-
269-
# Update metadata with enhanced message
270-
enhanced_metadata = metadata.copy()
271-
enhanced_metadata["clean_message"] = clean_message
272-
273-
# Convert to MarimoSQLError using converter
274-
return metadata_to_sql_error(enhanced_metadata)
251+
return MarimoSQLError(
252+
msg=str(exception),
253+
sql_statement=sql_statement,
254+
hint=None,
255+
sql_line=None,
256+
sql_col=None,
257+
)

marimo/_sql/sql.py

Lines changed: 5 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -80,43 +80,14 @@ def sql(
8080
df = sql_engine.execute(query)
8181
except Exception as e:
8282
if is_sql_parse_error(e):
83-
# Use centralized error processing
84-
from marimo._sql.error_utils import (
85-
create_sql_error_metadata,
86-
)
87-
88-
metadata = create_sql_error_metadata(
89-
e,
90-
rule_code="runtime",
91-
node=None,
92-
sql_content=query,
93-
context="sql_execution",
94-
)
95-
96-
# Enhance error messages based on exception type
97-
exception_type = metadata["error_type"]
98-
clean_message = metadata["clean_message"]
99-
if exception_type == "ParserException":
100-
clean_message = f"SQL syntax error: {clean_message}"
101-
elif "ParseError" in exception_type:
102-
clean_message = f"SQL parse error: {clean_message}"
103-
elif "ProgrammingError" in exception_type:
104-
clean_message = f"SQL programming error: {clean_message}"
105-
106-
# Truncate long SQL statements
107-
truncated_query = (
108-
query[:200] + "..." if len(query) > 200 else query
109-
)
110-
111-
# Raise MarimoSQLException with structured hint data
11283
# NB. raising _from_ creates a noisier stack trace, but preserves
11384
# the original exception context for debugging.
11485
raise MarimoSQLException(
115-
message=clean_message,
116-
sql_statement=truncated_query,
117-
sql_line=metadata["sql_line"],
118-
sql_col=metadata["sql_col"],
119-
hint=metadata["hint"],
86+
message=str(e),
87+
sql_statement=query,
88+
sql_line=None,
89+
sql_col=None,
90+
hint=None,
12091
) from e
12192
raise
12293

tests/_sql/test_sql_error_handling.py

Lines changed: 7 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,7 @@ def test_long_sql_statement_truncation(self):
9898
sql(long_query)
9999

100100
error = exc_info.value
101-
# Should be truncated to ~200 chars + "..."
102-
assert len(error.sql_statement) <= 203
103-
if len(long_query) > 200:
104-
assert error.sql_statement.endswith("...")
101+
assert len(error.sql_statement) == len(long_query)
105102

106103

107104
class TestSQLGlotParseErrors:
@@ -231,11 +228,7 @@ def __init__(self, sql_statement: str):
231228
except Exception as e:
232229
mock_cell = MockCell(long_statement)
233230
error = create_sql_error_from_exception(e, mock_cell)
234-
235-
# Should be truncated
236-
assert len(error.sql_statement) <= 203
237-
if len(long_statement) > 200:
238-
assert error.sql_statement.endswith("...")
231+
assert len(error.sql_statement) == len(long_statement)
239232

240233

241234
class TestErrorMessageQuality:
@@ -264,23 +257,6 @@ def test_extract_sql_position_no_position(self):
264257
assert line is None
265258
assert col is None
266259

267-
@pytest.mark.skipif(not HAS_DUCKDB, reason="DuckDB not installed")
268-
def test_error_message_enhancement(self):
269-
"""Test that error messages are enhanced with prefixes."""
270-
import duckdb
271-
272-
class MockCell:
273-
sqls = ["SELECT * FRM invalid"]
274-
275-
try:
276-
duckdb.sql("SELECT * FRM invalid")
277-
except Exception as e:
278-
error = create_sql_error_from_exception(e, MockCell())
279-
# Should have "SQL syntax error:" prefix for ParserException
280-
assert error.msg.startswith(
281-
"SQL syntax error:"
282-
) or error.msg.startswith("SQL parse error:")
283-
284260
def test_error_message_cleaning(self):
285261
"""Test that error messages are cleaned of traces."""
286262

@@ -293,8 +269,7 @@ class MockCell:
293269

294270
error = create_sql_error_from_exception(MockException(), MockCell())
295271
# Should only contain the first line, no traceback
296-
assert "Traceback" not in error.msg
297-
assert error.msg == "SQL error message"
272+
assert "Traceback" in error.msg
298273

299274

300275
class TestIntegrationAndEdgeCases:
@@ -369,10 +344,7 @@ def test_duckdb_hints_preserved(self):
369344
# Check that the main error message is present
370345
assert "does not exist" in error_msg
371346
# Check that the hint is properly extracted to the hint field
372-
assert error.hint is not None
373-
assert (
374-
"Did you mean" in error.hint or "candidate" in error.hint.lower()
375-
)
347+
assert error.hint is None
376348

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

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

417388
# Verify the struct has the hint field and it's populated
418389
assert hasattr(error_struct, "hint")
419-
assert error_struct.hint is not None
420-
assert (
421-
"Did you mean" in error_struct.hint
422-
or "candidate" in error_struct.hint.lower()
423-
)
424-
# Main message should not contain the hint
425-
assert error_struct.hint not in error_struct.msg
390+
assert error_struct.hint is None
426391

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

448413
# Verify multiline hint is captured completely
449414
assert hasattr(error_struct, "hint")
450-
assert error_struct.hint is not None
451-
assert "Candidate functions:" in error_struct.hint
452-
assert "substring(VARCHAR, BIGINT, BIGINT)" in error_struct.hint
453-
assert "substring(VARCHAR, BIGINT)" in error_struct.hint
454-
# Should be multiline
455-
assert "\n" in error_struct.hint
456-
# Main message should be clean
457-
assert "Candidate functions:" not in error_struct.msg
415+
assert error_struct.hint is None

0 commit comments

Comments
 (0)