reporting::CheckedImpl::RetrieveClusterData crashes when attribute read fails

[bzbarsky-apple]

Reproduction steps

scripts/examples/gn_build_example.sh examples/energy-management-app/linux out/debug chip_config_network_layer_ble=false chip_use_data_model_interface='"check"'

Then run ./out/debug/chip-energy-management-app, commission it, and do:

chip-tool any read-by-id 0x94 0x00 ${NODE_ID} 1

The app crashes. This happens because that attribute read fails in both the "data model" and "ember" versions of read, but the "ember" version does the equivalent of AttributeReportBuilder::PrepareAttribute before it tries the part that fails, while the "data model" version does them in the opposite order. So while statusEmber != statusDm tests false, lengthWrittenEmber != reportBuilder.GetWriter()->GetLengthWritten() is true and we hit the fatal assert.

There's a comment there that says:

    // NOTE: RetrieveClusterData is responsible for encoding StatusIB errors in case of failures
    //       so we validate length written requirements for BOTH success and failure. 

but that's just not true. The StatusIB encoding is done by the caller of RetrieveClusterData (Engine::BuildSingleReportDataAttributeReportIBs) after rolling back the TLV in error cases. Except for IsOutOfSpaceEncodingResponse() situations.

So presumably either the logic in Read-Checked needs to be fixed to be looser in its assertions or the two RetrieveClusterData implementations need to be changed to produce equivalent output as this code expects.

Bug prevalence

Always

GitHub hash of the SDK that was being used

ebc4237

Platform

core

Platform Version(s)

No response

Anything else?

No response

[bzbarsky-apple]

Disabling this in #35307 for now to work around SVE problems.

[andy31415]

Reproduced via python script by:

    attr = await devCtrl.ReadAttribute(
        node_id, [Clusters.WaterHeaterManagement.Attributes.HeaterTypes]
    )

(wrote a test script in https://github.com/andy31415/chip-repl-tests/blob/main/energy_management_read.py).
Looking into fixing this

[andy31415]

It seems the logic in the code states that we do not compare data length for non-unit-tests because it is not reliable (time-dependent sizes may differ). However the logic for this seems broken as CONFIG_BUILD_FOR_HOST_UNIT_TEST seems to be enabled when building the energy management app. This seems odd.