fix: ensure manifest file_size is updated

[riderx]

Summary

Fixed on_manifest_create trigger to ensure file_size is always updated in the database. Previously, when getSize returned 0, the update was skipped, leaving file_size as 0 in production. Now it throws an error to trigger queue retry.

Test plan

• Queue system will retry on size=0 until getSize returns valid size
• When size > 0, database update succeeds and function returns success
• Logs show file_size_zero errors when S3 returns 0 size

Checklist

• Change has adequate logging for debugging
• Queue retry mechanism ensures reliability

Summary by CodeRabbit

• Bug Fixes
  • Manifest file-size handling improved: zero-size files now log record context (id and path) and raise a retryable error.
  • Non-zero sizes follow a single, consistent update path to set file size; failures during update now surface explicit errors and are logged for clearer failure diagnosis.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

The manifest creation trigger now treats zero file sizes as retryable errors (logs id and s3_path then throws), and otherwise always updates the manifest's file_size in the database; update failures are explicitly logged and throw a manifest_update_failed error.

Changes

Cohort / File(s)	Summary
Size handling logic in manifest trigger supabase/functions/_backend/triggers/on_manifest_create.ts	Added explicit zero-size validation that logs id and s3_path and throws a retryable file_size_zero error. After retrieving size, always update manifest.file_size; on update failure log and throw manifest_update_failed with record and error.

Sequence Diagram(s)

sequenceDiagram
    participant Trigger as "Manifest Create Trigger"
    participant Storage as "Object Storage (S3)"
    participant DB as "Database"
    participant Logger as "Logger"

    Trigger->>Storage: fetch object metadata / size (s3_path)
    Storage-->>Trigger: return size
    alt size == 0
        Trigger->>Logger: log retryable zero-size (id, s3_path)
        Trigger-->>DB: throw file_size_zero (retry)
    else size > 0
        Trigger->>DB: update manifest.file_size = size
        DB-->>Trigger: update result / error
        alt update error
            Trigger->>Logger: log manifest_update_failed (record, updateError)
            Trigger-->>DB: throw manifest_update_failed
        end
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 I nibble logs beneath the moonlit code,

Found a zero where the bytes abode.
I logged its name and gave a hop — retry!
Then set the size and watched the manifest sigh. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly summarizes the main change: ensuring the manifest file_size field is updated, which is the core fix addressed in this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@supabase/functions/_backend/triggers/on_manifest_create.ts`:
- Around line 26-28: The updateError branch currently only logs and returns
success, swallowing DB errors; change the handler in on_manifest_create so that
when updateError is truthy you log with cloudlog (including requestId via
c.get('requestId')) and then throw the updateError (or a new Error wrapping it)
to force the queue to retry; also add braces around the if (updateError) block
since it will contain multiple statements.

[Copilot]

Pull request overview

This PR modifies the on_manifest_create trigger to handle cases where S3's getSize returns 0 by throwing an error to trigger queue retry, instead of silently skipping the file_size update.

Changes:

• Inverted the conditional logic to explicitly check for size === 0 and throw an error for queue retry
• Database update now executes unconditionally when size > 0 (previously was inside a conditional block)
• Added logging when getSize returns 0 before triggering retry

[Copilot]

When the database update fails (updateError is truthy), the error is logged but the function continues and returns success (c.json(BRES)). This could leave the database in an inconsistent state where the manifest record exists but the file_size was not updated. Consider throwing an error when updateError is truthy to ensure the queue retries the operation until the database update succeeds.

Suggested change

	if (updateError)
	cloudlog({ requestId: c.get('requestId'), message: 'error update manifest size', error: updateError })
	if (updateError) {
	cloudlog({ requestId: c.get('requestId'), message: 'error update manifest size', error: updateError })
	throw simpleError('manifest_update_failed', 'Failed to update manifest file_size', { record, updateError })
	}

[Copilot]

The current implementation will cause an infinite retry loop if a manifest file legitimately has a size of 0 bytes. While such files might be rare, they are technically valid. The getSize function returns 0 both when there's an error fetching the size (line 123 in s3.ts) and when file.size is actually 0 (line 119 in s3.ts). Consider adding a retry counter or maximum retry limit to prevent infinite loops, or differentiate between "file not ready yet" (should retry) vs "file is actually 0 bytes" (should accept and update).

[riderx]

@copilot queue system who call this function has already the max retry

[Copilot]

The new size=0 retry behavior lacks test coverage. The existing test in tests/trigger-error-cases.test.ts only covers missing app_version_id or s3_path validation. Consider adding test cases for: 1) when getSize returns 0 (should return 400 with 'file_size_zero' error), and 2) when getSize returns a valid size > 0 (should successfully update the database).

[Copilot]

@riderx I've opened a new pull request, #1467, to work on those changes. Once the pull request is ready, I'll request review from you.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud