Skip to content

store_entity drops entities on two error paths; load_entity can leave the page cache enabled #183

Description

@LukasGold

Two pre-existing gaps in src/osw/core.py, found while working on #175. Neither is caused by that issue or its fix.

1. store_entity_ drops an entity on two error paths

store_entity_, the worker inside OSW.store_entity, logs an error and returns on two conditions:

  • src/osw/core.py:1856-1863 - get_title(entity_) raises
  • src/osw/core.py:1866-1875 - namespace_ or title_ is None

A plain return is not an exception. handle_upload_object_ therefore returns normally, results receives None for that entity, and the collector loop at src/osw/core.py:2048-2052 adds nothing to failed. The entity reaches neither StoreEntityResult.pages nor StoreEntityResult.failed.

Consequences:

  • store_entity does not raise StoreEntityPartialError, so the caller sees a successful call
  • len(result.pages) is smaller than the number of entities passed in, with no indication why
  • only the log records the failure

The third error condition in the same function, raise TypeError(...) at src/osw/core.py:1877, behaves correctly: it propagates and the collector loop records it in failed.

Expected

Both conditions raise, so the entity is reported in StoreEntityResult.failed like every other failure.

2. load_entity does not restore the page cache state when a call raises

OSW.load_entity saves the cache state at src/osw/core.py:1223, changes it at src/osw/core.py:1225 and src/osw/core.py:1228, and restores it at src/osw/core.py:1312-1316. The restore is not in a finally block.

Calls between the two points that can raise and are not caught:

  • src/osw/core.py:1231 - self.site.get_page(...)
  • src/osw/core.py:1238 - page.get_slot_content("jsondata")
  • src/osw/core.py:1244-1252 - fetching each referenced category schema
  • src/osw/core.py:1261-1267 - self.fetch_schema(...)
  • src/osw/core.py:1301-1309 - namespace_from_full_title / title_from_full_title

If any of these raises, the cache stays in the state set for the duration of the load. A caller that had the cache disabled then keeps it enabled for the rest of the process. That is the same failure mode as #176, where a later get_page can return a revision from before the write.

Expected

The restore runs in a finally block.

Note

_fetch_schema has the same unprotected pattern at src/osw/core.py:599-600 and src/osw/core.py:1061. That one is already fixed in #180 and is not part of this issue.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions