Skip to content

Raise on failed system POST instead of swallowing it - #50

Merged
tipatterson-dev merged 2 commits into
mainfrom
fix/insert-self-silent-failure
Jul 28, 2026
Merged

Raise on failed system POST instead of swallowing it#50
tipatterson-dev merged 2 commits into
mainfrom
fix/insert-self-silent-failure

Conversation

@tipatterson-dev

Copy link
Copy Markdown
Collaborator

Fixes #42.

What was wrong

System.insert_self() assigned _resource_id only when res.ok, and did nothing otherwise — no raise, no log. Node.add_system(insert_resource=True) then returned normally, so the caller believed the system existed server-side. The failure surfaced much later as AttributeError: 'System' object has no attribute '_resource_id', typically from add_insert_datastream(), with no trace of the original HTTP error.

_resource_id was only created in System.__init__ when a resource_id kwarg was passed, so a from_resource()-built system didn't have the attribute at all — any pre-insert access was an AttributeError rather than a clean is None check.

Changes

resources/system.pyinsert_self() raises on a non-ok response, with label, URN, status code, and response body. The message shape matches the four sibling add_insert_* methods that already raise.

resources/base.pyStreamableResource.__init__ sets self._resource_id = None unconditionally, fixing all three wrappers at once. System, Datastream, and ControlStream all assign their real id after super().__init__(), so nothing is clobbered. This also repairs System.retrieve_resource(), which already guarded on if self._resource_id is None and therefore died before reaching that check on a from_resource()-built system.

node.pyadd_system docstring records that a failed POST propagates and the system is not attached.

docs/source/architecture/insertion.md — documents the raise.

Tests

Three added, 479 passing:

  • test_insert_self_raises_on_failed_post — the raise carries both status code and response body
  • test_resource_id_is_none_before_insertNone on both construction paths, and retrieve_resource() returns cleanly instead of raising
  • test_add_system_does_not_attach_on_failed_insert — the issue's actual repro: raises, and leaves nothing in node.systems()

Compatibility

insert_self() now raises where it previously returned. Callers that relied on the silent return were, by construction, proceeding with a system that had no server-side id — the failure mode this fixes. The raise is a plain Exception, matching the rest of the insert path; #47 tracks promoting all five sites to a typed hierarchy.

Version bumped 0.5.2a10.5.3a1 (behavior fix, not routine iteration).

Follow-ups filed

Found in the same pass, not fixed here: #43, #44, #45, tracked with #47/#48/#49 under the umbrella #46.

insert_self() left _resource_id unset on a non-ok response, so the
failure only surfaced later as an AttributeError; _resource_id is now
also initialized to None on every streamable. Fixes #42.
@tipatterson-dev
tipatterson-dev merged commit 6dc75ee into main Jul 28, 2026
14 checks passed
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.

System.insert_self() silently swallows failed POSTs, causing downstream AttributeError

1 participant