Skip to content

fix: include column context in bulk load validation errors - #1827

Open
Ananya2 wants to merge 2 commits into
tediousjs:masterfrom
Ananya2:fix/bulk-load-validation-error-context
Open

Ananya2 wants to merge 2 commits into
tediousjs:masterfrom
Ananya2:fix/bulk-load-validation-error-context

Conversation

@Ananya2

@Ananya2 Ananya2 commented Oct 13, 2026 •

Copy link
Copy Markdown

When a value in a bulk load fails type validation, the error doesn't say which column it belongs to. With many columns and many rows, the only clue is a generic message such as Invalid date., so the bad value is hard to find.

Change

Validation failures in BulkLoadPayload are now wrapped in an InputError that names the failing column. The original validator error is kept as cause:

try {
  value = c.type.validate(input, c.collation);
} catch (error) {
  throw new InputError(`Column '${c.name}' could not be validated`, { cause: error });
}

This follows the convention Tedious already uses for the same kind of failure:

Bulk load validation was the remaining path that still threw the validator's error directly.

Before

TypeError: Invalid date.

After

InputError: Column 'createdAt' could not be validated
  [cause]: TypeError: Invalid date.

Sensitive values

The issue originally suggested attaching the rejected value to the error. This PR doesn't do that: the value could be a password, token or personal data, and errors usually end up in logs.

  • The message contains only the column name.
  • The rejected value is not stored on the error.
  • The original validator error is passed through unchanged as cause. Tedious's built-in validators use generic messages (Invalid date., Invalid number.), so they don't include the value. A custom data type that puts the value in its own error message would still carry it in cause.

Compatibility

  • InputError extends TypeError, so existing instanceof TypeError checks still pass.
  • The top-level message changes from the validator's text to Column '<name>' could not be validated. The original text is still available in error.cause.message.
  • No new public API.

Tests

  • Unit (test/unit/bulk-load-test.ts): new tests for both row shapes:

    • array rows, where the value is found by position
    • object rows, where the property name (objName) differs from the column name, so the test can check that the error names the database column rather than the property

    Each test checks the error type and the message, that cause is the original error, and that the rejected value appears neither in the error's stack nor as a property on the error.

  • Integration (test/integration/bulk-load-test.ts): two existing streaming validation tests expected the bare Invalid date. error. They now expect the wrapped InputError with the original error in cause.

@codecov

codecov Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.73%. Comparing base (3c98797) to head (91d660d).
⚠️ Report is 9 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1827      +/-   ##
==========================================
+ Coverage   83.37%   83.73%   +0.36%     
==========================================
  Files          95       96       +1     
  Lines        4709     4723      +14     
  Branches      881      884       +3     
==========================================
+ Hits         3926     3955      +29     
+ Misses        529      511      -18     
- Partials      254      257       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@arthurschreiber arthurschreiber left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this follows the existing InputError convention nicely. The main suggestion is to add the row index, as the TVP path already does, since the column name alone doesn't help much on large loads. A null row still fails with a context-free error, and one integration test lost its cause check. The rest are small test and style points.


Generated by Claude Code

Comment thread src/bulk-load-payload.ts Outdated
Comment thread src/bulk-load-payload.ts
Comment thread test/integration/bulk-load-test.ts Outdated
Comment thread test/unit/bulk-load-test.ts Outdated
Comment thread src/bulk-load-payload.ts
Comment thread test/unit/bulk-load-test.ts

@cheenamalhotra cheenamalhotra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few things worth addressing:

  1. Breaking change for error matching. Bulk load validation errors now have a different message (Column 'x' could not be validated at row index N instead of e.g. Invalid date.), and serialization errors gain at row index N. Code matching on err.message will break. Since release notes come from the squash commit, consider adding a footer so it's flagged:

    BREAKING CHANGE: Bulk load validation and serialization errors are now
    `InputError`s with messages like "Column 'x' could not be validated at
    row index N". The original error is in `err.cause`. Rows that are not
    arrays or objects now throw instead of loading as NULLs.
    

    @arthurschreiber — would you treat this as breaking, or just call it out in the notes?

  2. Rows that aren't objects now fail. A string or number row used to load silently as all NULLs. Good fix, but it's a behavior change. Please add a test with a primitive row (e.g. 'abc', 42).

  3. Sensitive value claim. writeDecimal (src/data-types/decimal-common.ts) throws Value ${value} is out of range..., which ends up in cause. Either drop the value from that message or adjust the claim in the description.

  4. Description is out of date. It doesn't mention the row index or the new row check. Please update the examples and the Compatibility section.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants