fix(customize): skip rows with a missing raw record instead of ending the scan

[vbhanuchander-lang]

Summary

The customize extractor cursor LEFT JOINs the raw table, so a domain row whose _raw_data_id points at a raw record that has since been deleted comes back with a NULL data column. That value fell through to the default branch of the type switch:

switch blob := row["data"].(type) {
case []byte:  ...
case string:  ...
default:
    return nil   // ends the whole scan
}

Nothing surfaced. The subtask reported success, and every row ordered after the first orphan silently kept whatever custom field values it already had — so a partially-populated table looks identical to a correctly-populated one. In the reported case 1,382 orphaned rows out of 30,287 left roughly 28,000 rows unpopulated.

This skips the row and carries on.

Change

• default now increments a counter and continues rather than returning.
• extractCustomizedFields returns how many rows it skipped for that reason, and ExtractCustomizedFields logs a warning naming the table, the raw table and the count. A summary rather than per-row logging, since the count can run into the thousands.
• The scan now returns rows.Err(). An error raised while iterating the cursor was previously discarded, which is the same class of silent partial processing and would be invisible in exactly the same way.

Skipping is the right behaviour rather than failing: an orphaned _raw_data_id means the raw record was cleaned up, which is a normal consequence of raw-data retention, not a corrupt state. Failing the subtask would make routine cleanup break every later sync.

Does this close any open issues?

Closes #8945

Screenshots

N/A — backend behaviour, no UI surface.

Other Information

Tests. The plugin had no unit tests, so this adds four, using the generated dal mocks to drive a cursor: orphans interleaved with valid rows (the reported shape), all rows orphaned, no orphans, and a cancelled context.

I checked the main test actually catches the bug rather than just passing — temporarily restoring the old return nil makes it fail with "[]" should have 2 item(s), but has 0, i.e. zero rows updated because the scan aborted at the first orphan. With the fix, both valid rows are updated and the two orphans are counted.

go test ./plugins/customize/tasks/ passes, gofmt and go vet clean for the changed files. The plugins/customize/e2e package fails locally for want of E2E_DB_URL, which is unrelated and excluded from scripts/unit-test-go.sh.

One thing I noticed but did not change. The []byte and string branches are not symmetric: the string branch has the issues + IsArray() handling that writes IssueCustomArrayField rows, and the []byte branch does not. Which branch a row takes depends on what the driver returns for the raw data column, so that feature may behave differently across databases. That felt like a separate question from this fix, and changing it could alter behaviour for existing users, so I left it alone — happy to open a follow-up issue if you would like it looked at.

[klesh]

I think it would be better to add the missing fields instead ignoring them.

[vbhanuchander-lang]

Thanks for looking @klesh. I may have described this poorly, because there aren't missing fields here — the raw record itself is gone.

The extractor cursor LEFT JOINs the raw table on _raw_data_id. When the referenced raw row no longer exists, data comes back NULL. There is no JSON to extract from, and nothing that could be added, because the source record was legitimately deleted.

It happens routinely rather than exceptionally. ApiCollector.Execute() deletes every raw row for the params at the start of any non-incremental collection and re-inserts them with fresh auto-increment ids:

// helpers/pluginhelper/api/api_collector.go
if !isIncremental {
    err = db.Delete(&RawData{}, dal.From(collector.table), dal.Where("params = ?", collector.params))
}

Any domain row still carrying an id from an earlier run is orphaned by that, so a single full re-collect is enough to produce this state.

If you meant clearing the custom fields rather than leaving them as they are — I did consider it, and I think it is worse: it would erase previously extracted values on every full re-collect, turning a routine operation into data loss. Leaving them and reporting the count seemed the safer default.

If you had something else in mind — for example re-linking domain rows to the new raw ids so the data can be re-extracted — I am happy to look at that, though it would belong in the collector rather than the customize plugin. Which would you prefer?