amadan

oficina / sheets Public

feat(xlsx): a typed import error that cannot leak a cell

Import's errors reach a background job's owner and, through
rastrillo's job runner, platform-held logs this app cannot grep
(spec §2.4). The coordinate in those errors was only available by
reading it back out of a string, which is fragile and also tempts a
caller into echoing the whole chain.

CellError carries the coordinate as data. Its Error() renders the
operation and the A1 ref and nothing else, so printing one cannot
leak a cell's contents even by accident; Unwrap still exposes the
cause, so errors.Is and errors.As are unaffected. The cause field is
unexported, which is what makes the guarantee structural rather than
remembered.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H6YuJ2QG6koRKjNf3ddr1Q
Paul Campbell pushed by paul@keymail.dev 1074b09f9bd9ed5dceedefe96ad6bca63590e0b7 parent 01e8adf
3 files changed, +87 −10
  • internal/sheets/xlsx/cellerror.go +41 −0
  • internal/sheets/xlsx/cellerror_test.go +37 −0
  • internal/sheets/xlsx/import.go +9 −10

Intent

full ledger: feat/xlsx-drop-import

diff --git a/internal/sheets/xlsx/cellerror.go b/internal/sheets/xlsx/cellerror.go
new file mode 100644
index 0000000..1b5ba1b
--- /dev/null
+++ b/internal/sheets/xlsx/cellerror.go
@@ -0,0 +1,41 @@
+package xlsx
+
+import "amadan.net/oficina/sheets/internal/sheets/calc"
+
+// CellError is an import failure that knows which cell it happened at.
+//
+// It exists because import runs as a background job whose error text
+// reaches both the owner and platform-held logs (spec §2.4, §11), and
+// the caller building that text needs the coordinate as DATA. Without a
+// type, the only way to name the failing cell is to parse it back out
+// of an error string — a fragile read that also tempts the caller into
+// echoing the whole chain, which is the leak this package spends its
+// error handling avoiding.
+//
+// Error() is safe by construction: it renders the operation and the
+// coordinate and nothing else. The wrapped cause is reachable through
+// Unwrap for errors.Is/As, so a sentinel still matches, but it is never
+// part of this error's own text — a caller that prints a CellError
+// cannot leak a cell's contents even by accident, which is a stronger
+// guarantee than a caller that remembers not to.
+type CellError struct {
+ // Ref is the A1-style coordinate: "B7".
+ Ref string
+ // Op is what was being attempted, phrased to read after "xlsx: "
+ // and before the coordinate: "cannot parse cell".
+ Op string
+ // err is the cause. Deliberately unexported and deliberately absent
+ // from Error(): it is the half that can carry content.
+ err error
+}
+
+func (e *CellError) Error() string { return "xlsx: " + e.Op + " " + e.Ref }
+
+// Unwrap exposes the cause to errors.Is and errors.As without putting
+// it in Error()'s text.
+func (e *CellError) Unwrap() error { return e.err }
+
+// cellErr builds a CellError for a 0-based position.
+func cellErr(op string, rowPos, colPos int, err error) error {
+ return &CellError{Ref: calc.A1(rowPos, colPos), Op: op, err: err}
+}
diff --git a/internal/sheets/xlsx/cellerror_test.go b/internal/sheets/xlsx/cellerror_test.go
new file mode 100644
index 0000000..3fa07e5
--- /dev/null
+++ b/internal/sheets/xlsx/cellerror_test.go
@@ -0,0 +1,37 @@
+package xlsx
+
+import (
+ "errors"
+ "strings"
+ "testing"
+)
+
+// TestCellErrorTextOmitsItsCause is the guarantee CellError exists for.
+//
+// Import's errors reach a background job's owner and platform-held logs
+// this app cannot grep (spec §2.4). A wrapped cause is the half that can
+// carry a cell's contents — a parse failure quoting the text it could
+// not parse, say — so CellError's own text must never include it, even
+// though Unwrap still hands it to errors.Is and errors.As.
+//
+// This is an internal test because the cause is unexported, which is
+// itself the point: nothing outside this package can put content into a
+// CellError's text, because nothing outside it can reach the field.
+func TestCellErrorTextOmitsItsCause(t *testing.T) {
+ const canary = "SECRET-PAYROLL-FIGURE-93f7"
+ cause := errors.New("cannot parse " + canary)
+
+ err := cellErr("cannot parse cell", 6, 1, cause)
+
+ if got := err.Error(); strings.Contains(got, canary) {
+ t.Fatalf("CellError text carries its cause: %q", got)
+ }
+ if got := err.Error(); !strings.Contains(got, "B7") {
+ t.Errorf("CellError text = %q, want the coordinate B7 in it", got)
+ }
+ // Unwrap still works, so a sentinel further down the chain is
+ // still matchable — the text is narrowed, not the error.
+ if !errors.Is(err, cause) {
+ t.Error("CellError does not unwrap to its cause")
+ }
+}
diff --git a/internal/sheets/xlsx/import.go b/internal/sheets/xlsx/import.go
index d39675b..f5b5b87 100644
--- a/internal/sheets/xlsx/import.go
+++ b/internal/sheets/xlsx/import.go
@@ -12,7 +12,6 @@ import (
"github.com/xuri/excelize/v2"
- "amadan.net/oficina/sheets/internal/sheets/calc"
"amadan.net/oficina/sheets/internal/sheets/store"
)
@@ -82,7 +81,7 @@ func Import(r io.Reader, h *store.Handle) error {
func importCell(f *excelize.File, sheetName string, h *store.Handle, rowPos, colPos int, raw string) error {
cellRef, err := excelize.CoordinatesToCellName(colPos+1, rowPos+1)
if err != nil {
- return fmt.Errorf("xlsx: cell %s: %w", calc.A1(rowPos, colPos), err)
+ return cellErr("cannot address cell", rowPos, colPos, err)
}
input, err := cellInputText(f, sheetName, cellRef, rowPos, colPos, raw)
@@ -92,11 +91,11 @@ func importCell(f *excelize.File, sheetName string, h *store.Handle, rowPos, col
styleID, err := f.GetCellStyle(sheetName, cellRef)
if err != nil {
- return fmt.Errorf("xlsx: read style at %s: %w", calc.A1(rowPos, colPos), err)
+ return cellErr("cannot read the style at", rowPos, colPos, err)
}
style, err := f.GetStyle(styleID)
if err != nil {
- return fmt.Errorf("xlsx: read style at %s: %w", calc.A1(rowPos, colPos), err)
+ return cellErr("cannot read the style at", rowPos, colPos, err)
}
format, hasFormat := formatFromStyle(style)
@@ -106,21 +105,21 @@ func importCell(f *excelize.File, sheetName string, h *store.Handle, rowPos, col
rowID, err := h.EnsureRow(rowPos)
if err != nil {
- return fmt.Errorf("xlsx: cell %s: %w", calc.A1(rowPos, colPos), err)
+ return cellErr("cannot make the row for cell", rowPos, colPos, err)
}
colID, err := h.EnsureCol(colPos)
if err != nil {
- return fmt.Errorf("xlsx: cell %s: %w", calc.A1(rowPos, colPos), err)
+ return cellErr("cannot make the column for cell", rowPos, colPos, err)
}
if input != "" {
if err := h.Set(rowID, colID, input, importActor); err != nil {
- return fmt.Errorf("xlsx: cannot parse cell %s: %w", calc.A1(rowPos, colPos), err)
+ return cellErr("cannot parse cell", rowPos, colPos, err)
}
}
if hasFormat {
if err := h.SetFormat(rowID, colID, format); err != nil {
- return fmt.Errorf("xlsx: cannot set format at %s: %w", calc.A1(rowPos, colPos), err)
+ return cellErr("cannot set the format at", rowPos, colPos, err)
}
}
return nil
@@ -142,7 +141,7 @@ func importCell(f *excelize.File, sheetName string, h *store.Handle, rowPos, col
func cellInputText(f *excelize.File, sheetName, cellRef string, rowPos, colPos int, raw string) (string, error) {
formula, err := f.GetCellFormula(sheetName, cellRef)
if err != nil {
- return "", fmt.Errorf("xlsx: read formula at %s: %w", calc.A1(rowPos, colPos), err)
+ return "", cellErr("cannot read the formula at", rowPos, colPos, err)
}
if formula != "" {
return "=" + formula, nil
@@ -153,7 +152,7 @@ func cellInputText(f *excelize.File, sheetName, cellRef string, rowPos, colPos i
cellType, err := f.GetCellType(sheetName, cellRef)
if err != nil {
- return "", fmt.Errorf("xlsx: read cell type at %s: %w", calc.A1(rowPos, colPos), err)
+ return "", cellErr("cannot read the cell type at", rowPos, colPos, err)
}
if cellType == excelize.CellTypeBool {
if raw == "1" {