From 9c0a1cf0a6dc94d648fc31e9e0701d6b6b7f1c26 Mon Sep 17 00:00:00 2001 From: kraysent Date: Sat, 25 Jul 2026 17:03:04 +0100 Subject: [PATCH 1/2] fix install script to support cygwin & more readable error --- frontend/src/api.ts | 1 + frontend/src/components/HistoryPage.tsx | 36 ++++++++++++++++++------ frontend/src/components/ProgressView.tsx | 25 ++++++++++++++-- scripts/install.sh | 11 +------- uploader/app/report.py | 1 + uploader/history.py | 1 + uploader/tasks.py | 16 +++++++---- 7 files changed, 65 insertions(+), 26 deletions(-) diff --git a/frontend/src/api.ts b/frontend/src/api.ts index dd9b644..04e69ab 100644 --- a/frontend/src/api.ts +++ b/frontend/src/api.ts @@ -67,6 +67,7 @@ export type HistoryEntry = { inputs: Record; status: "success" | "error" | "cancelled"; message: string; + details?: string | null; }; export async function fetchHistory(): Promise { diff --git a/frontend/src/components/HistoryPage.tsx b/frontend/src/components/HistoryPage.tsx index a7bb49c..5270799 100644 --- a/frontend/src/components/HistoryPage.tsx +++ b/frontend/src/components/HistoryPage.tsx @@ -161,14 +161,34 @@ export function HistoryPage() { - - {entry.message} + + + {entry.message} + + {entry.details && ( + + + Details + + {entry.details} + + )} diff --git a/frontend/src/components/ProgressView.tsx b/frontend/src/components/ProgressView.tsx index f78072e..150d976 100644 --- a/frontend/src/components/ProgressView.tsx +++ b/frontend/src/components/ProgressView.tsx @@ -17,7 +17,7 @@ type StreamEvent = caption: string | null; timestamp: string; } - | { type: "error"; message: string } + | { type: "error"; message: string; details: string } | { type: "done"; message: string } | { type: "cancelled"; message: string }; @@ -53,6 +53,7 @@ export function ProgressView({ const [done, setDone] = useState(null); const [cancelled, setCancelled] = useState(null); const [error, setError] = useState(null); + const [errorDetails, setErrorDetails] = useState(null); const [cancelPending, setCancelPending] = useState(false); const [cancelError, setCancelError] = useState(null); const logContainerRef = useRef(null); @@ -98,6 +99,7 @@ export function ProgressView({ }); } else if (ev.type === "error") { setError(ev.message); + setErrorDetails(ev.details); es.close(); } else if (ev.type === "cancelled") { setCancelled(ev.message); @@ -137,10 +139,29 @@ export function ProgressView({ {percent}% {error && ( - + {error} )} + {errorDetails && ( + + + Details + + {errorDetails} + + )} {cancelled && ( str: run = TaskRun(run_id=run_id) final_status: history.HistoryStatus | None = None final_message: str = "" + final_details: str | None = None def append_report_event(event: report.Event) -> None: - nonlocal final_status, final_message + nonlocal final_status, final_message, final_details match event: case report.LogEvent(message=msg): out = _log_message_with_time(msg) @@ -102,7 +103,7 @@ def append_report_event(event: report.Event) -> None: final_status = "success" final_message = msg run.append({"type": "done", "message": msg}) - case report.ErrorEvent(message=msg): + case report.ErrorEvent(message=msg, details=details): logger.error( "error event", task_id=task_id, @@ -110,7 +111,8 @@ def append_report_event(event: report.Event) -> None: ) final_status = "error" final_message = msg - run.append({"type": "error", "message": msg}) + final_details = details + run.append({"type": "error", "message": msg, "details": details}) case report.ImageEvent(data_url=url, caption=cap): run.append( { @@ -127,7 +129,7 @@ def report_func(event: report.Event) -> None: append_report_event(event) def worker() -> None: - nonlocal final_status, final_message + nonlocal final_status, final_message, final_details token = action_description.set_current( action_description.build(task_id, run_id, form.model_dump(mode="json")), ) @@ -138,8 +140,9 @@ def worker() -> None: final_message = "Task was cancelled by user." run.append({"type": "cancelled", "message": final_message}) except Exception as e: - message = f"{e}\n\n{traceback.format_exc()}" - append_report_event(report.ErrorEvent(message=message)) + append_report_event( + report.ErrorEvent(message=str(e), details=traceback.format_exc()), + ) finally: action_description.reset_current(token) run.done.set() @@ -152,6 +155,7 @@ def worker() -> None: inputs=form_data, status=final_status, message=final_message, + details=final_details, ), ) From b38b9bb0e0a8561d9db9c7b311021dc72b3627b9 Mon Sep 17 00:00:00 2001 From: kraysent Date: Sat, 25 Jul 2026 17:11:13 +0100 Subject: [PATCH 2/2] better error messages --- tests/test_geometry_upload.py | 127 +++++++++++++++++++++ uploader/app/structured/geometry/upload.py | 35 +++++- uploader/app/structured/icrs/upload.py | 48 +++++++- 3 files changed, 201 insertions(+), 9 deletions(-) create mode 100644 tests/test_geometry_upload.py diff --git a/tests/test_geometry_upload.py b/tests/test_geometry_upload.py new file mode 100644 index 0000000..1504736 --- /dev/null +++ b/tests/test_geometry_upload.py @@ -0,0 +1,127 @@ +from unittest.mock import Mock, patch + +import pytest + +from uploader.app.structured.geometry.upload import upload_geometry_isophotal +from uploader.clients.gen.client import adminapi + + +def _mock_storage(total: int = 1) -> Mock: + storage = Mock() + storage.query.return_value = [{"cnt": total}] + return storage + + +def _mock_client() -> Mock: + return Mock(spec=adminapi.AuthenticatedClient) + + +def _base_expressions() -> dict[str, str]: + return { + "a": '3 * 10 ** col("logd25") * arcsec', + "e_a": '3 * 10 ** col("logd25") * 2.302585093 * e_logd25 * arcsec', + "b": '3 * 10 ** (col("logd25") - col("logr25")) * arcsec', + "e_b": ( + '3 * 10 ** (col("logd25") - col("logr25")) * 2.302585093 ' + '* (col("e_logd25") ** 2 + col("e_logr25") ** 2) ** 0.5 * arcsec' + ), + "isophote": 'col("bri25")', + } + + +@patch("uploader.app.structured.geometry.upload.rawdata_batches") +@patch("uploader.app.structured.geometry.upload._fetch_column_units") +def test_isophote_unit_conversion_error_includes_field_details( + mock_fetch_column_units: Mock, + mock_rawdata_batches: Mock, +) -> None: + mock_fetch_column_units.return_value = ( + {"logd25", "logr25", "e_logd25", "e_logr25", "bri25"}, + { + "logd25": "", + "logr25": "", + "e_logd25": "", + "e_logr25": "", + "bri25": "", + }, + ) + mock_rawdata_batches.return_value = iter( + [ + [ + { + "hyperleda_internal_id": "000079ce-5ffd-82c6-3f75-3a083f0fde80", + "logd25": 1.5, + "logr25": 0.3, + "e_logd25": 0.05, + "e_logr25": 0.04, + "bri25": 25.0, + }, + ], + ], + ) + + with pytest.raises(RuntimeError, match="failed to evaluate expressions for row") as exc_info: + upload_geometry_isophotal( + _mock_storage(), + "test_table", + "B", + _base_expressions(), + 100, + _mock_client(), + report_func=lambda _: None, + ) + + message = str(exc_info.value) + assert "isophote" in message + assert 'col("bri25")' in message + assert "mag/arcmin2" in message + assert "columns: bri25=''" in message + + +@patch("uploader.app.structured.geometry.upload.rawdata_batches") +@patch("uploader.app.structured.geometry.upload._fetch_column_units") +def test_constant_isophote_unit_error_omits_empty_columns( + mock_fetch_column_units: Mock, + mock_rawdata_batches: Mock, +) -> None: + mock_fetch_column_units.return_value = ( + {"logd25", "logr25", "e_logd25", "e_logr25"}, + { + "logd25": "", + "logr25": "", + "e_logd25": "", + "e_logr25": "", + }, + ) + mock_rawdata_batches.return_value = iter( + [ + [ + { + "hyperleda_internal_id": "000079ce-5ffd-82c6-3f75-3a083f0fde80", + "logd25": 1.5, + "logr25": 0.3, + "e_logd25": 0.05, + "e_logr25": 0.04, + }, + ], + ], + ) + expressions = _base_expressions() + expressions["isophote"] = "22" + + with pytest.raises(RuntimeError, match="failed to evaluate expressions for row") as exc_info: + upload_geometry_isophotal( + _mock_storage(), + "test_table", + "B", + expressions, + 100, + _mock_client(), + report_func=lambda _: None, + ) + + message = str(exc_info.value) + assert "isophote" in message + assert "('22')" in message + assert "from dimensionless to mag/arcmin2" in message + assert "columns:" not in message diff --git a/uploader/app/structured/geometry/upload.py b/uploader/app/structured/geometry/upload.py index f7e39a9..0c81ac3 100644 --- a/uploader/app/structured/geometry/upload.py +++ b/uploader/app/structured/geometry/upload.py @@ -156,14 +156,40 @@ def _validate_columns( raise RuntimeError(f"Table {table_name} has no column(s): {missing}") +def _format_unit(unit: u.UnitBase) -> str: + text = f"{unit:s}".strip() + return text if text else "dimensionless" + + +def _eval_context_suffix(expr: Expression, column_units: dict[str, str]) -> str: + if not expr.referenced_columns: + return "" + parts = [f"{col}={column_units.get(col, '')!r}" for col in sorted(expr.referenced_columns)] + return f"; columns: {', '.join(parts)}" + + def _evaluate_field( expr: Expression, values: dict[str, float], column_units: dict[str, str], field: str, + source: str, ) -> float: - quantity = expr.evaluate(values, column_units).to(u.Unit(TARGET_UNITS[field])) - return float(quantity.value) + target = TARGET_UNITS[field] + try: + quantity = expr.evaluate(values, column_units) + except (ValueError, u.UnitConversionError, u.UnitTypeError) as e: + raise RuntimeError( + f"failed to evaluate {field!r} ({source!r}){_eval_context_suffix(expr, column_units)}: {e}", + ) from e + try: + return float(quantity.to(u.Unit(target)).value) + except (u.UnitConversionError, u.UnitTypeError) as e: + raise RuntimeError( + f"failed to convert {field!r} ({source!r}) " + f"from {_format_unit(quantity.unit)} to {target}" + f"{_eval_context_suffix(expr, column_units)}: {e}", + ) from e def upload_geometry_isophotal( @@ -216,9 +242,10 @@ def upload_geometry_isophotal( values = {col: float(row[col]) for col in needed_cols} try: evaluated = { - field: _evaluate_field(expr, values, column_units, field) for field, expr in parsed.items() + field: _evaluate_field(expr, values, column_units, field, expressions[field]) + for field, expr in parsed.items() } - except (ValueError, u.UnitConversionError, u.UnitTypeError) as e: + except RuntimeError as e: raise RuntimeError( f"failed to evaluate expressions for row {row['hyperleda_internal_id']}: {e}", ) from e diff --git a/uploader/app/structured/icrs/upload.py b/uploader/app/structured/icrs/upload.py index db98acc..ea8fb42 100644 --- a/uploader/app/structured/icrs/upload.py +++ b/uploader/app/structured/icrs/upload.py @@ -79,14 +79,40 @@ def _parse_expressions(expressions: dict[str, str]) -> dict[str, Expression]: return {field: parse(source) for field, source in expressions.items()} +def _format_unit(unit: u.UnitBase) -> str: + text = f"{unit:s}".strip() + return text if text else "dimensionless" + + +def _eval_context_suffix(expr: Expression, column_units: dict[str, str]) -> str: + if not expr.referenced_columns: + return "" + parts = [f"{col}={column_units.get(col, '')!r}" for col in sorted(expr.referenced_columns)] + return f"; columns: {', '.join(parts)}" + + def _evaluate_error_field( expr: Expression, values: dict[str, float], column_units: dict[str, str], field: str, + source: str, ) -> float: - quantity = expr.evaluate(values, column_units).to(u.Unit(TARGET_ERROR_UNITS[field])) - return float(quantity.value) + target = TARGET_ERROR_UNITS[field] + try: + quantity = expr.evaluate(values, column_units) + except (ValueError, u.UnitConversionError, u.UnitTypeError) as e: + raise RuntimeError( + f"failed to evaluate {field!r} ({source!r}){_eval_context_suffix(expr, column_units)}: {e}", + ) from e + try: + return float(quantity.to(u.Unit(target)).value) + except (u.UnitConversionError, u.UnitTypeError) as e: + raise RuntimeError( + f"failed to convert {field!r} ({source!r}) " + f"from {_format_unit(quantity.unit)} to {target}" + f"{_eval_context_suffix(expr, column_units)}: {e}", + ) from e def _fetch_column_units( @@ -169,9 +195,21 @@ def upload_icrs( values = {col: float(row[col]) for col in error_cols} try: - e_ra_val = _evaluate_error_field(parsed["e_ra"], values, column_units, "e_ra") - e_dec_val = _evaluate_error_field(parsed["e_dec"], values, column_units, "e_dec") - except (ValueError, u.UnitConversionError, u.UnitTypeError) as e: + e_ra_val = _evaluate_error_field( + parsed["e_ra"], + values, + column_units, + "e_ra", + expressions["e_ra"], + ) + e_dec_val = _evaluate_error_field( + parsed["e_dec"], + values, + column_units, + "e_dec", + expressions["e_dec"], + ) + except RuntimeError as e: raise RuntimeError( f"failed to evaluate expressions for row {row['hyperleda_internal_id']}: {e}", ) from e