Skip to content

Strip file_error and rescue unknown attributes in raw data archive import - #3564

Merged
Freika merged 1 commit into
masterfrom
detail/bug-fix/strip-file-error-and-rescue-unknown-attributes-in-a3921c
Sep 10, 2026
Merged

Strip file_error and rescue unknown attributes in raw data archive import#3564
Freika merged 1 commit into
masterfrom
detail/bug-fix/strip-file-error-and-rescue-unknown-attributes-in-a3921c

Conversation

@detail-app

@detail-app detail-app Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Detail bug report: View on Detail

Bug

Users::ImportData::RawDataArchives aborted the entire user-data restore when a single raw_data_archives.jsonl row carried a file_error key.

  • Users::ExportData#add_archive_file_data writes a file_error key into a row whenever the archive's blob can't be downloaded/decrypted/written (missing blob, rotated encryption key, S3 outage, disk full, etc.).
  • Points::RawDataArchive has no file_error column, so mass-assigning it raised ActiveModel::UnknownAttributeError.
  • The importer's rescue clause only caught ActiveRecord::RecordInvalid, so the exception escaped to Users::ImportData#import's top-level ActiveRecord::Base.transaction, rolling back every entity imported earlier in V2Handler#process (settings, areas, places, tags, taggings, imports, exports, trips, stats, digests, notifications, visits, tracks, points). The user saw a generic "unknown attribute 'file_error' for Points::RawDataArchive" failure notification with no indication that one bad raw_data_archive row was the cause.

The sibling importers (imports.rb, exports.rb) already handle this exact case by stripping file_error and rescuing ActiveModel::UnknownAttributeError; raw_data_archives.rb was the only one missing both.

Fix

In app/services/users/import_data/raw_data_archives.rb, mirror the sibling importers:

  • create_archive_record: added 'file_error' to the archive_data.except(...) list so the exporter's error marker is not mass-assigned.
  • call rescue clause: broadened rescue ActiveRecord::RecordInvalid to rescue ActiveRecord::RecordInvalid, ActiveModel::UnknownAttributeError so an unknown attribute from a future/desynced exporter skips just that row instead of aborting the whole import.

Testing

Added 4 specs to spec/services/users/import_data/raw_data_archives_spec.rb mirroring the existing file_error pattern in exports_spec.rb:

  • A file_error row still creates the archive (with the key stripped) and restores no file.
  • A file_error row shaped like real production output (no file_name) doesn't raise.
  • A mixed batch (valid row, file_error row, valid row) imports all rows without aborting.
  • A row carrying an unrelated unknown attribute is skipped and surrounding rows still import (guards the rescue clause itself).

Verification:

  • The new spec file passes (10 examples, 0 failures).
  • Negative control performed: temporarily reverted both edits and confirmed all 4 new examples fail with exactly ActiveModel::UnknownAttributeError: unknown attribute 'file_error' for Points::RawDataArchive. (bogus_column for the rescue-clause test); re-applied the fix and confirmed all 10 pass.
  • Sibling-spec regression: spec/services/users/import_data/ — 256 examples, 0 failures.
  • Exporter regression: spec/services/users/export_data_spec.rb — 23 examples, 0 failures (the file_error emission path at lines 173-214 still produces the input the importer must now tolerate).
  • Round-trip integration: spec/services/users/export_import_integration_spec.rb — 8 examples, 0 failures.
  • V1 path regression: spec/services/users/import_data/v1_handler_spec.rb — 17 examples, 0 failures.
  • Code quality: bundle exec rails zeitwerk:check clean; bundle exec rubocop on both changed files reports no offenses.

End-to-end V2 round-trip with a real file_error row (export a user whose raw_data_archive blob was deleted from storage, then import into a fresh user via Users::ImportData#import) was attempted via a Rails runner script against the test database, but blocked by FactoryBot/RSpec-mock setup not being available outside the RSpec context in the runner; the same scenario is covered by the unit specs above combined with the existing exporter spec that confirms file_error rows are emitted, so the round-trip behavior is verified at the component level.


Automatic Fixes PRs can be configured here.

@detail-app
detail-app Bot requested a review from Freika September 7, 2026 10:01
@Freika
Freika merged commit 7be7b0d into master Sep 10, 2026
19 checks passed
@Freika
Freika deleted the detail/bug-fix/strip-file-error-and-rescue-unknown-attributes-in-a3921c branch September 10, 2026 20:16
pull Bot pushed a commit to AmirulAndalib/dawarich that referenced this pull request Sep 13, 2026
Follow-up to Freika#3564, which merged before these review fixes landed.

A file_error row became an archive record with no file attached. The verifier
then reports it as unattached, and find_existing_archive treats the slot as
taken, so a later export that does carry the file can never fill it. Those rows
are now skipped with a warning naming the year, month and chunk.

Attributes are sliced to the model's own columns, as the points importer
already does, so a column added on the exporting instance is ignored per key
rather than dropping every archive row through the rescue.
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.

1 participant