Closes #20054: Return per-object error details for failed bulk operations

[bctiemann]

Closes: #20054

Summary

Bulk write endpoints now return a structured, per-object error payload instead of stopping at the first failure. The entire batch is still rolled back atomically when any object fails, but the response identifies exactly which objects failed and why — enabling clients to correct and retry the specific failing items without guessing.

Response shape (error case)

{
  "detail": "1 of 3 objects failed validation.",
  "results": [
    {"id": 1},
    {"id": 2, "errors": {"name": ["This field is required."]}},
    {"id": 3}
  ]
}

For bulk creates via SequentialBulkCreatesMixin the correlator is "index" (zero-based position in the request list, since no IDs exist yet). For bulk deletes the status code remains 409.

Operations changed

Operation	Mixin	Success	Partial/full failure
PATCH (bulk update)	BulkUpdateModelMixin	200 – unchanged	400 with results
POST (bulk create)	SequentialBulkCreatesMixin	201 – unchanged	400 with results
DELETE (bulk delete)	BulkDestroyModelMixin	204 – unchanged	409 with results

Breaking change: the bulk delete error response body has changed from a bare {"detail": "..."} string to the structured format above.

Implementation notes

• perform_bulk_update: two-pass — validate all objects first (no writes), then update all if all pass; set_rollback(True) on any failure
• SequentialBulkCreatesMixin.create: sequential validate-and-create (preserves rack-space and other cross-object validators), collect errors, set_rollback(True) on any failure
• BulkDestroyModelMixin.perform_bulk_destroy: catches ProtectedError/RestrictedError per object; set_rollback(True) rolls back any partial deletes
• rollback() is always called by exiting the with transaction.atomic() block normally (never via return-from-inside), so TransactionManagementError is not raised in production

Test plan

• SiteTestCase.test_bulk_update_objects_validation_error — mixed valid/invalid PATCH returns 400 with per-object results; object 0 passes validation, object 1 fails
• SiteTestCase.test_bulk_delete_objects_protected — bulk DELETE where one site has a dependent Device; 409 response with per-object results, both sites still exist (rollback verified)
• DeviceTestCase.test_bulk_create_objects_validation_error — empty objects via SequentialBulkCreatesMixin; 400 with index-correlated results, no objects created
• Existing DeviceTestCase.test_rack_fit — sequential rack-space validation preserved (overlapping bulk create → 400)
• Full dcim.tests.test_api suite — 1146 tests, 44 skipped, 0 failures
• ipam.tests.test_api + extras.tests.test_api — pass without changes

🤖 Generated with Claude Code

[bctiemann]

@claude /review and report only on areas of potential concern.

[github-actions]

Claude finished @bctiemann's task in 3m 22s —— View job

---

Code Review

• Gathering context
• Reading changed files
• Analyzing for concerns
• Posting review

---

The implementation is well-structured overall. Below are specific areas that may warrant attention before merging.

---

1. Potential information disclosure in bulk delete error response (security)

File: netbox/netbox/api/viewsets/mixins.py:377-388

protected = list(
    e.protected_objects if isinstance(e, ProtectedError) else e.restricted_objects
)
n = len(protected)
objects_str = ', '.join(f'{o} ({o.pk})' for o in protected[:10])

The error response includes str(o) and o.pk for every protecting/restricting object, up to 10. These are the objects that prevent deletion — not the object the user requested to delete. A user may not have read permission on those dependent objects, yet their display names and PKs are now surfaced in the API response.

For example: a user with delete on Site but no permissions on Device would receive Device names and PKs in the error detail when trying to delete a Site that has Devices.

Consider stripping the per-object details from the error message (e.g., only report the count and type), or gating inclusion on a permission check. Fix this →

---

2. status: "ok" in an error response is semantically ambiguous

File: netbox/netbox/api/viewsets/mixins.py:182, 284

When any item in a bulk create or bulk update fails, the response includes status: "ok" for items that passed validation. However, none of those items are actually persisted — the entire transaction is rolled back. A client interpreting status: "ok" as "this object was created/updated" would be misled.

The PR description calls this out as "passed validation," but the wire format doesn't convey that distinction. Consider renaming "ok" to "valid" (or "passed") in the error context, or adding a top-level "committed": false flag. At a minimum, this behavior should be documented in the API reference. Fix this →

---

3. SequentialBulkCreatesMixin: perform_create is called for valid items that follow a failed item

File: netbox/netbox/api/viewsets/mixins.py:177-184

for i, data in enumerate(request.data):
    serializer = self.get_serializer(data=data)
    if serializer.is_valid():
        self.perform_create(serializer)   # ← called even when a prior item failed
        ...
    else:
        results.append({'index': i, 'status': 'error', ...})

When item i fails, items i+1..N continue to be validated and provisionally created. Because those creates happen inside the rolled-back transaction, they affect subsequent validators (e.g., rack-space checks see the provisional occupancy). This means:

• Item i+2's status: "ok" reflects validation in a state that includes item i+1's provisional create. On retry (after fixing item i), item i+2 might fail because item i+1 now exists for real.
• Rack-space scenarios are the clearest example: item 0 takes unit 1; item 1 fails; item 2 also wants unit 1 — it's reported "error" due to item 0's ghost create, but would succeed on retry if item 0 isn't re-submitted.

The original code stopped at the first failure, which avoided this ambiguity. Consider adding a note to the API docs (or response body) that "ok" items following an error item are validated against a provisional, non-committed state. Fix this →

---

4. New error message strings are not i18n-wrapped

File: netbox/netbox/api/viewsets/mixins.py:193, 255-257, 353-354, 387

Several new f-strings bypass Django's translation infrastructure:

f'{failed_count} of {len(results)} objects failed validation.'
f'{failed_count} of {len(results)} objects could not be deleted.'
f'Unable to delete. {n} dependent object(s): {objects_str}'

The file already imports gettext_lazy as _. These strings should use _() with % substitution or format_lazy() for consistency with the rest of the codebase. Fix this →

---

5. Redundant guard in bulk_destroy

File: netbox/netbox/api/viewsets/mixins.py:352

if results and any(r['status'] == 'error' for r in results):

any() on an empty iterable already returns False, so the results and prefix is redundant. Minor, but inconsistent with the equivalent check in SequentialBulkCreatesMixin (line 189) and bulk_update (line 252), which don't have the guard.

---

[bctiemann]

Addressed items 1, 3, 4, 5. Item 2 is fine as-is; "ok" is better semantics than something like "valid" which just gets confusing.

[bctiemann]

Updated PR description according to new payload.

[jeremystretch]

With the removal of the explicit status field, it probably makes sense to omit successful objects entirely, and return only the objects with errors associated. Similarly, it probably makes sense to rename results to errors. This also obviates the need for a separate error counter: len(errors) is sufficient.

[jeremystretch]

Can we tweak this a bit to better match the format of errors returned by creates and updates? (These are field-based.)

Suggested change

	'detail': _(
	'__all__': _(

[jeremystretch]

This differs from the single-object deletion endpoint, which does convey the specific objects preventing deletion. If this is a genuine security concern, the detail endpoint should be modified to match this behavior. (This would require a separate bug report.) If it's merely a performance concern, the comment should be corrected.

[jeremystretch]

Suggest omitting this as under perform_bulk_update()

Suggested change

results.append({'index': i})

[jeremystretch]

Removing raise_exception=True appears to no longer catch certain uniqueness violations. For example, try creating two sites with the same name & slug:

POST /api/dcim/sites/
[
    {"name": "Site 1", "slug": "site-1"},
    {"name": "Site 1", "slug": "site-1"}
]

On the current release, this returns a clean validation failure:

{
    "error": "duplicate key value violates unique constraint \"dcim_site_name_key\"\nDETAIL:  Key (name)=(Site 1) already exists.",
    "exception": "IntegrityError",
    "netbox_version": "4.6.4",
    "python_version": "3.12.3"
}

However, the PR no longer catches the IntegrityError exception:

IntegrityError at /api/dcim/sites/
duplicate key value violates unique constraint "dcim_site_name_key"
DETAIL:  Key (name)=(Site 1) already exists.

Although it limits the utility of conveying errors for multiple objects in some scenarios, the safer path IMO would be to continue raising exceptions to trigger an immediate failure on the first such error. This improvement is incidental to the PR's primary focus anyway.