More robust error handling and improved ergonomics in case of unreadable data

[tmadlener]

Currently, when podio doesn't know how to instantiate a collection buffer for a collection on file, we just default construct a CollectionReadBuffer, which will not have a properly initialized create function, and will ultimately result in a bad_function_call, e.g.

terminate called after throwing an instance of 'std::bad_function_call'
  what():  bad_function_call
/<prefix>/bin/podio-dump: line 37: 2647243 Aborted                 (core dumped) "${THIS_DIR}"/podio-dump-tool "${ALL_ARGS[@]}"

The implementations are (among other places)

podio/src/ROOTReader.cc

Lines 149 to 153 in 6bd80b0

	const auto& bufferFactory = podio::CollectionBufferFactory::instance();
	auto maybeBuffers = bufferFactory.createBuffers(collType, schemaVersion, isSubsetColl);
	// TODO: Error handling of empty optional
	auto collBuffers = maybeBuffers.value_or(podio::CollectionReadBuffers{});

podio/src/RNTupleReader.cc

Lines 180 to 182 in 6bd80b0

	const auto& bufferFactory = podio::CollectionBufferFactory::instance();
	const auto maybeBuffers = bufferFactory.createBuffers(collType, coll.schemaVersion, coll.isSubset);
	const auto collBuffers = maybeBuffers.value_or(podio::CollectionReadBuffers{});

podio/python/templates/SIOBlock.cc.jinja2

Lines 19 to 23 in 6bd80b0

	const auto& bufferFactory = podio::CollectionBufferFactory::instance();
	// TODO:
	// - Error handling of empty optional
	auto maybeBuffers = bufferFactory.createBuffers("{{ class.full_type }}Collection", sio::version::major_version(version), m_subsetColl);
	m_buffers = maybeBuffers.value_or(podio::CollectionReadBuffers{});

The least we should do is to give a more informative error message here.

[tmadlener]

After #868 and #863 there is the following (example) output:

WARNING: Buffers couldn't be created for collection GeneratorEventParameters of type edm4hep::GeneratorEventParametersCollection and schema version 6
WARNING: Buffers couldn't be created for collection MCParticles of type edm4hep::MCParticleCollection and schema version 6
WARNING: Buffers couldn't be created for collection SimCalorimeterHitContributions of type edm4hep::CaloHitContributionCollection and schema version 6
WARNING: Buffers couldn't be created for collection SimCalorimeterHits of type edm4hep::SimCalorimeterHitCollection and schema version 6
WARNING: Buffers couldn't be created for collection SimTrackerHits of type edm4hep::SimTrackerHitCollection and schema version 6
WARNING: Buffers couldn't be created for collection TPCHits of type edm4hep::RawTimeSeriesCollection and schema version 6
WARNING: Buffers couldn't be created for collection TrackerHitPlanes of type edm4hep::TrackerHitPlaneCollection and schema version 6
terminate called after throwing an instance of 'std::runtime_error'
  what():  Cannot retrieve collection MCParticles of type edm4hep::MCParticleCollection

It gives at least some more information than bad_function_call. I consider this good enough for now. We can re-open or create a new issue if there is need for further improvements.

[noorule39-hub]

Hi @tmadlener, I’m a C++ developer interested in contributing to PODIO. I’ve been reviewing the context from #847 and would like to help implement the preventative version check discussed here in #848.

My proposed approach:
-- I'll look into src/ROOTReader.cc and src/RNTupleReader.cc to intercept the EDM version during the initial metadata loading phase.
-- If the file version exceeds the version PODIO was built with (podio::version::build_version), I'll implement a std::runtime_error to catch the mismatch before it leads to the bad_function_call issues noted in the parent ticket.
-- I’ve noticed the 'TODO' comments in the reader implementations regarding empty optional handling; I’m happy to tackle those as part of this logic to ensure we don't just warn, but fail safely.
-- Could you assign this to me? Also, if there are any specific 'future-version' sample files you'd like me to use for testing, please let me know.

[tmadlener]

Hi @noorule39-hub,

In principle your plan should work (with a few minor adjustments). However, I just discovered that I had already done some work in this direction, which I never put into a PR it seems. I have now done that with #935. So I am afraid that the interesting parts of this issue have already been done.

If you are still interested in contributing, I think #647 would also be a nice issue to tackle that is well defined (+some discussion of how to approach it from a technical point of view). Let me know if that would be interesting for you (with a comment there).