fix: fail the task chain when time-series retrieval fails (#2063)

[jcschaff]

Fixes #2063. Takes the inverted-default approach rather than the contained one.

What was wrong

PDEDataViewer.TimeSeriesDataRetrievalTask caught every exception, recorded it under
StringKey_timeSeriesJobException and returned normally:

} catch (Exception exception) {
    hashTable.put(StringKey_timeSeriesJobException, exception);
}   // never rethrown

Four of its six callers never read that key. The dispatcher therefore treated the task as
successful and ran the next one against a null result. What the user saw was an NPE in an
unrelated plotting task, with the real cause thrown away. A cancellation went the same way,
since UserCancelException was swallowed along with everything else.

call site	before
PdeTimePlotMultipleVariablesPanel:201	NPE — the reported crash
PDEDataViewer:1177	NPE — plotSpaceStats dereferences getVariableNames() immediately
PDEDataViewer:3013	same NPE via plotSpaceStats
PDEDataViewer:3072	opened a save-file chooser first, so the user named a movie file for data that was never retrieved, then failed
PDEDataViewer:2624	checked, and did nothing but rethrow
KymographPanel:2035	checked, and handled it locally

The fix

Throwing is now the default. ClientTaskDispatcher already does exactly the right thing with a
thrown exception — recordException routes UserCancelException to TASK_ABORTED_BY_USER and
stays quiet, while anything else becomes TASK_ABORTED_BY_ERROR and an error dialog naming the
real cause — so the four broken callers are fixed without touching them, including the
save-file-chooser ordering at 3072.

KymographPanel is the reason a blanket rethrow would have been wrong. It reports failures
inside the panel via failMethod() and also folds in TASK_ABORTED_BY_USER; rethrowing would
have skipped task3 and lost that handling entirely. It opts back in through a new
TimeSeriesDataRetrievalTask.recordingFailure(...) factory.

The matching check in PDEDataViewer:2624's task is now unreachable, so it is removed rather
than left implying the key can still be set.

One thing that made this safe to reason about: the TimeSeriesDataJobListener DATA_COMPLETE /
DATA_FAILURE hashtable writes are commented out, so the synchronous task is the only writer
of either key — there is no async race here.

Tests

New TimeSeriesDataRetrievalTaskTest (5 tests, Fast): a failure and a cancellation both abort
the chain carrying the original cause, recordingFailure() still records instead of throwing,
a success publishes its results, and the data-job listener is removed however the task ends.

I checked them against the old behaviour rather than assuming they bite — reverting just the
throw makes 3 of the 5 fail, and the 2 that pass are the 2 that should (recordingFailure
and the success path).

Verification

• TimeSeriesDataRetrievalTaskTest — 5/5, and 3/5 fail without the fix
• vcell-client Fast — 36 tests, 0 failures, 0 errors
• full mvn compile test-compile across all modules — clean

Not covered by automated tests: the kymograph's in-panel failure message and the dispatcher's
error dialog are Swing paths. The kymograph call site is a one-line change to the factory with
its downstream logic untouched, so the risk is confined to that panel still receiving the
recorded exception — which the test asserts directly.

🤖 Generated with Claude Code

https://claude.ai/code/session_01D71LBYmQNf5J94wPqr81Jx