uploadDataset silently uploads zero documents and never writes the cloud link — plus the wider except Exception swallow sweep it came from #130

Follow-up to #95 / #128. That PR fixed two functions that reported an AttributeError as an ordinary answer. This is the sweep for the same pattern elsewhere, and the first thing it found is worse than either of the originals.

1. uploadDataset uploads nothing and reports success

ndi.cloud.orchestration reaches for dataset.session in five places. ndi_dataset has no session attribute — measured, not assumed:

hasattr(ndi_dataset_dir("myds", tmp), "session")   -> False
hasattr(ndi.session.dir("ref", tmp), "session")    -> False

Unlike database_fun.py (which tries session_or_dataset.database_search(q) first and only falls back to .session.…, so its dead path is unreachable and harmless), orchestration.py goes straight to dataset.session.… with no primary attempt. Four of the five swallow the resulting AttributeError into an empty list or a no-op.

Run against a real dataset with only the network stubbed:

documents actually in the dataset : 4
documents uploadDataset sent      : 0
result                            : ok=True cloud_id='NEW-CLOUD-ID'
dataset_remote link written back  : (False, '')

Three separate failures, all silent:

• Zero documents are uploaded. orchestration.py:429 gathers all_docs through dataset.session.database_search(...), gets [], and uploads an empty collection.
• It reports success. (True, cloud_id, ""), so nothing downstream can tell.
• The dataset_remote link is never written back. orchestration.py:418 does dataset.session.database_add(remote_doc) under except Exception: pass. The dataset stays unlinked, so is_in_cloud() is still False and the next upload creates another remote dataset — the exact duplication getCloudDatasetIdForLocalDataset is dead for every real dataset; port dataset.isInCloud() #95 predicted, reached by a different route.

This is reachable from the GUI: gui/nav/datasets_cloud.py:190 is the navigator's "Upload to Cloud" action.

The five sites

Fix: the same one #128 applied — search through dataset.database_search / dataset.database_add, matching MATLAB, and stop swallowing. Site 599 shows what the others should look like.

Test: with a real ndi_dataset and the cloud API stubbed, assert uploadDataset sends as many documents as the dataset holds and that is_in_cloud() is True afterwards. Every current test mocks these paths out, which is why three failures in one function went unnoticed.

2. The sweep

An AST pass over src/ for broad handlers (except Exception, except BaseException, bare except:) whose body is a single pass/continue or a single default assignment:

172 total. Of those, 58 guard an attribute access on a name — the exact shape that produced #128 and item 1 above.

Only ndi.cloud.orchestration has been examined so far — that is where the sweep stopped, because item 1 was enough to file on. The remaining 167 are unaudited, and the count is not a defect count: many are legitimate.

Sorting them

Not every swallow is wrong, and the sweep should not turn into a mass rewrite. Three categories:

• Fails safe — e.g. filesNotYetUploaded assumes every file needs uploading when it cannot check. Wrong answer, harmless direction. Leave, or add a debug log.
• Guarded — database_fun.py's .session fallback is dead code, but it runs only after the correct call already failed. Harmless; worth deleting for clarity, not urgent.
• Fails silent and wrong — Fix two dataset lookups that silently reported the wrong answer #128's two, and the four here. These are the target.

The distinguishing question is the one that catches all of them: does this handler let a programming error (a missing attribute, a wrong path) return a value that the caller cannot tell apart from a real answer? An unreachable database is not an empty dataset; a missing attribute is not "never uploaded".

Suggested guard

tests/test_every_module_imports.py catches modules that are broken on their face. Nothing catches a function that is broken on its face while its module imports fine. A test in that spirit — exercising the cloud entry points against a real ndi_dataset with only the HTTP layer stubbed — would have caught #128 and all of item 1, and is a better long-term answer than auditing 172 handlers by hand.

Effort

Item 1 is small and self-contained (src/ndi/cloud/orchestration.py, five lines plus tests) and should not wait for the sweep. Item 2 is open-ended and best done package by package, cloud first since it is where both confirmed instances live.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CqrNhfGnhKGcQvNiP2LNXK