Skip to content

Subject write errors crash the program #339

Description

@rly

Note on provenance: This issue was found by Claude Opus 5 while reviewing #320. The reproductions and output shown below were produced by building and running the code, but I have only briefly reviewed the analysis myself and I am not an expert in this area. Please review it critically.

Impact

Two ordinary situations kill the process instead of returning an error the caller can handle: writing subject metadata where a field is an empty string, and writing a subject to a file that already has one.

A caller that wraps the call in a normal try/catch still terminates, because H5::Exception does not derive from std::exception:

try {
  nwbfile->initialize(uuid, desc, coll, t, t, std::optional(spec));
} catch (const std::exception& e) {
  // never reached, the process terminates
}

An acquisition program losing a recording session to a blank text field is a bad failure mode for a library whose job is to not lose data.

What happens

The scalar overload of HDF5IO::createStringDataSet (src/io/hdf5/HDF5IO.cpp:1254) has no try/catch, so an HDF5 failure travels straight out through Subject::initialize and NWBFile::initialize. The vector overload just below it already catches and returns Status::Failure.

An empty string field. SubjectSpec fields are std::optional<std::string>, so an empty string is a value that is set, not a value that is absent. Writing subjectSpec.sex = "" is a natural way to say "this value is not known", and it is easy to reach when subject metadata comes from a config file, a GUI text box, or a database column that returned nothing. All ten branches in Subject::initialize test only has_value() (src/nwb/file/Subject.cpp:55 and following), so the empty string reaches HDF5 as a string type of size zero, which HDF5 rejects.

A subject that already exists. Calling Subject::initialize a second time, or calling it on a file opened in append mode that already contains /general/subject, tries to create datasets that are already there. NWBFile::initialize guards this with an objectExists check (src/nwb/NWBFile.cpp:106), so it reaches users through the Subject class directly, which #320 makes public.

Observed exceptions
empty string       H5::Exception  func=StrType::setSize        detail=H5Tset_size failed
already exists     H5::Exception  func=H5File::createDataSet   detail=H5Dcreate2 failed

Suggested direction

Catch H5::Exception in the scalar createStringDataSet and return Status::Failure, matching the vector overload. That closes both triggers and protects every other caller of that function.

Then skip fields whose value is empty, the way NWBFile::createFileStructure already handles its own optional string dataset at src/nwb/NWBFile.cpp:197, so an unknown value is simply not written.

Shape of the field guard, repeated for each field
if (subjectSpec.sex.has_value() && !subjectSpec.sex->empty()) {
  ...
}

Given this repeats ten times, a small helper taking the field and its dataset name would keep initialize readable.

Activity

  1. added
    category: bugerrors in the code or code behavior
    priority: highimpacts proper operation or use of a feature important to most users
    topic: apiissues related to the core AqNWB API
    priority: mediumnon-critical problem and/or affecting only a small set of users
    and removed
    priority: highimpacts proper operation or use of a feature important to most users
    on Sep 3, 2026
  2. added
    priority: lowalternative solution already working and/or relevant to only specific user(s)
    and removed
    priority: mediumnon-critical problem and/or affecting only a small set of users
    on Sep 3, 2026
  3. added this to the 0.5.0 milestone on Sep 3, 2026
  4. oruebel commented on Sep 3, 2026

    @oruebel
    Contributor

    Also related to #338 which can be fixed together

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

    category: bugerrors in the code or code behaviorpriority: lowalternative solution already working and/or relevant to only specific user(s)topic: apiissues related to the core AqNWB API

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions