mirror of
https://github.com/gristlabs/grist-core.git
synced 2024-10-27 20:44:07 +00:00
(core) Add CellError to explain when a formula refers to a cell with an error value
Summary: get_cell_value wraps RaisedException with CellError to expand the error message for the user. This is still pretty conceptual, the comments explain some things to think about, but it works and is an improvement. Test Plan: Updated Python unit tests Reviewers: dsagal, paulfitz Reviewed By: dsagal Differential Revision: https://phab.getgrist.com/D2928
This commit is contained in:
parent
1605e18f66
commit
0e5c2bee59
@ -147,7 +147,12 @@ class BaseColumn(object):
|
||||
"""
|
||||
raw = self.raw_get(row_id)
|
||||
if isinstance(raw, objtypes.RaisedException):
|
||||
if isinstance(raw.error, depend.CircularRefError):
|
||||
# Wrapping a CircularRefError in a CellError is often redundant, but
|
||||
# TODO a CellError is still useful if the calling cell is not involved in the cycle
|
||||
raise raw.error
|
||||
else:
|
||||
raise objtypes.CellError(self.table_id, self.col_id, row_id, raw.error)
|
||||
if self.type_obj.is_right_type(raw):
|
||||
return self._make_rich_value(raw)
|
||||
return usertypes.AltText(str(raw), self.type_obj.typename())
|
||||
|
@ -12,6 +12,7 @@ import six
|
||||
import column
|
||||
from functions import date # pylint: disable=import-error
|
||||
from functions.unimplemented import unimplemented
|
||||
from objtypes import CellError
|
||||
from usertypes import AltText # pylint: disable=import-error
|
||||
from records import Record, RecordSet
|
||||
|
||||
@ -597,6 +598,9 @@ def _prepare_record_dict(record, dates_as_iso=False, expand_refs=0):
|
||||
result[col_id] = val
|
||||
except Exception as e:
|
||||
result[col_id] = None
|
||||
while isinstance(e, CellError):
|
||||
# The extra information from CellError is redundant here
|
||||
e = e.error # pylint: disable=no-member
|
||||
errors[col_id] = "%s: %s" % (type(e).__name__, str(e))
|
||||
|
||||
if errors:
|
||||
|
@ -263,11 +263,18 @@ class RaisedException(object):
|
||||
# and to DB (for when we store formula results). There are two concerns: one is that it's
|
||||
# potentially quite verbose; the other is that it's makes the tests more annoying (again b/c
|
||||
# verbose).
|
||||
error = self.error
|
||||
location = ""
|
||||
while isinstance(error, CellError):
|
||||
if not location:
|
||||
location = "\n(in referenced cell {error.location})".format(error=error)
|
||||
error = error.error
|
||||
name = type(error).__name__
|
||||
if self.details:
|
||||
return [type(self.error).__name__, str(self.error), self.details]
|
||||
if isinstance(self.error, InvalidTypedValue):
|
||||
return [type(self.error).__name__, self.error.typename, self.error.value]
|
||||
return [type(self.error).__name__]
|
||||
return [name, str(error) + location, self.details]
|
||||
if isinstance(error, InvalidTypedValue):
|
||||
return [name, error.typename, error.value]
|
||||
return [name]
|
||||
|
||||
@classmethod
|
||||
def decode_args(cls, *args):
|
||||
@ -275,6 +282,24 @@ class RaisedException(object):
|
||||
return cls(None, encoded_error=list(args))
|
||||
|
||||
|
||||
class CellError(Exception):
|
||||
def __init__(self, table_id, col_id, row_id, error):
|
||||
super(CellError, self).__init__(table_id, col_id, row_id, error)
|
||||
self.table_id = table_id
|
||||
self.col_id = col_id
|
||||
self.row_id = row_id
|
||||
self.error = error
|
||||
|
||||
def __str__(self):
|
||||
return (
|
||||
"{self.error.__class__.__name__} in referenced cell {self.location}"
|
||||
).format(self=self)
|
||||
|
||||
@property
|
||||
def location(self):
|
||||
return "{self.table_id}[{self.row_id}].{self.col_id}".format(self=self)
|
||||
|
||||
|
||||
class RecordList(list):
|
||||
"""
|
||||
Just like list but allows setting custom attributes, which we use for remembering _group_by and
|
||||
|
@ -330,9 +330,18 @@ else:
|
||||
self.assertFormulaError(self.engine.get_formula_error('AttrTest', 'B', 1),
|
||||
AttributeError, "Table 'AttrTest' has no column 'AA'",
|
||||
r"AttributeError: Table 'AttrTest' has no column 'AA'")
|
||||
self.assertFormulaError(self.engine.get_formula_error('AttrTest', 'C', 1),
|
||||
AttributeError, "Table 'AttrTest' has no column 'AA'",
|
||||
r"AttributeError: Table 'AttrTest' has no column 'AA'")
|
||||
cell_error = self.engine.get_formula_error('AttrTest', 'C', 1)
|
||||
self.assertFormulaError(cell_error,
|
||||
objtypes.CellError, "AttributeError in referenced cell AttrTest[1].B",
|
||||
r"CellError: AttributeError in referenced cell AttrTest\[1\].B")
|
||||
self.assertEqual(
|
||||
objtypes.encode_object(cell_error),
|
||||
['E',
|
||||
'AttributeError',
|
||||
"Table 'AttrTest' has no column 'AA'\n"
|
||||
"(in referenced cell AttrTest[1].B)",
|
||||
cell_error.details]
|
||||
)
|
||||
|
||||
def test_cumulative_formula(self):
|
||||
formula = ("Table1.lookupOne(A=$A-1).Principal + Table1.lookupOne(A=$A-1).Interest " +
|
||||
|
Loading…
Reference in New Issue
Block a user