NASA-AMMOS / aerie

A software framework for modeling spacecraft.
https://nasa-ammos.github.io/aerie-docs/
MIT License
73 stars 19 forks source link

Null "parent" values in serialized activities break command expansion #1346

Closed cartermak closed 8 months ago

cartermak commented 9 months ago

Checked for duplicates

Yes - I've already checked

Is this a regression?

No - This is a new bug

Version

v2.2.X

Describe the bug

The Command Expansion activity loader fails to deserialize activities where "container" value schema types (struct, series) have null corresponding values.

I'm capturing this as a bug since the Aerie UI seems to have no problem handling this, and in my opinion it makes sense for any value to be null and for Command Expansion to handle that case/forward that value to the expansion rule. A move towards disallowing these null values may require significant rework of several complex Clipper activity parameters/mappers.

Reproduction

One example: Clipper's Optional parameter type serializes the value as null if the optional is empty. This issue can be reproduced by simulating an activity with this parameter type as "empty" where the optional value is itself a list or struct.

For example, given this value schema for an Optional parameter with a complex "struct" value:

{
  "type": "struct",
  "items": {
    "value": {
      "type": "struct",
      "items": {
        "softwareMode": {
          "type": "struct",
          "items": {
            "value": {
              "type": "variant",
              "variants": [
                {
                  "key": "OFF",
                  "label": "OFF"
                },
                {
                  "key": "ON",
                  "label": "ON"
                }
              ]
            },
            "present": {
              "type": "boolean"
            }
          }
        },
        "hardwareState": {
          "type": "variant",
          "variants": [
            {
              "key": "OFF",
              "label": "OFF"
            },
            {
              "key": "ON",
              "label": "ON"
            }
          ]
        }
      }
    },
    "present": {
      "type": "boolean"
    }
  }
}

...the serialized parameter value below will cause the issue in the sequencing server:

{
  "value": null,
  "present": false
}

Logs

"stack": "TypeError: Cannot read properties of null (reading 'softwareMode')
    at convertType (file:///app/build/lib/batchLoaders/simulatedActivityBatchLoader.js:138:57)
    at convertType (file:///app/build/lib/batchLoaders/simulatedActivityBatchLoader.js:138:40)
    at file:///app/build/lib/batchLoaders/simulatedActivityBatchLoader.js:105:32
    at Array.reduce (<anonymous>)
    at mapGraphQLActivityInstance (file:///app/build/lib/batchLoaders/simulatedActivityBatchLoader.js:102:78)
    at file:///app/build/lib/batchLoaders/simulatedActivityBatchLoader.js:38:81
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Promise.all (index 2)
    at async Promise.all (index 0)"

System Info

Clipper Aerie deployment on Aerie v2.2.X, multiple hosts.

Severity

Major

Workaround

Branch plan, delete activities with null values, run expansions on branch. This is not always a realistic option, but may work for some plans

cartermak commented 9 months ago

@joswig I didn't see the your edit to this last week; I don't think that workaround will suffice since the problem activities have meaningful modeling which will change the generated commands and/or are part of decompositions that also include command expansion, but I'll keep it in the back of my mind in case we need to go that route.