Fix: Handle undefined value in performance attribute Jira-OPG-492

[sbernabe1]

Description:
This pull request fixes a TypeError that occurred when processing data from the Performance data source. The previous code was attempting to access a property on an undefined variable, which would cause the plugin to fail.

The fix ensures the variable is correctly checked for a value before being used, preventing the error.

Release-9.x

Please Review:
@synqotik
@marshallmassengill

[synqotik]

@sbernabe1 @dino2gnt @marshallmassengill Thanks for finding this issue.

Could someone confirm what value for propertyValue is being passed to setPerformanceStateNode when this TypeError occurs?

Is it null or undefined? Then we can check if (!propertyValue) and return. This could be a stopgap measure just to get around this issue for now.

If it is something else, but it's not a PerformanceAttributeItemState, then it could mean that Grafana v12 has changed their API and somehow a different value is being passed. This conversion may be occurring elsewhere, so we would need to do some investigation.

Also if this worked before, but now the value is always null or undefined then it probably also mean Grafana changed their API and we aren't mapping the value correctly and thus losing data.

In any case we should not need to stringify/parse the object here, you'll get a TypeError anyway.

[sbernabe1]

@synqotik, thanks for the detailed questions! I've done some more testing and have a few updates on what's going on.

First, to answer your question: the value of propertyValue being passed is indeed undefined when the TypeError first occurs. Your check of if (!propertyValue) is an attempt to handle that initial error.

However, I found that even with that check, I was still getting a TypeError on a subsequent call, even though the propertyValue was an OnmsNode object. It seems that the object is a complex type that the function isn't expecting.

You were right that we shouldn't have to stringify/parse the object. But in this case, it was the only way to successfully "sanitize" the object by creating a shallow copy containing only the data the function needed. This allows the rest of the function to execute without error and properly process the node's properties.

The current code in the pull request now includes both the if (!propertyValue) check and the JSON.parse(JSON.stringify(...)) fix, which addresses the root cause of the TypeError.

[synqotik]

@sbernabe1 thanks for the info!

The issue might be that in OnmsNode, id is a number instead of a string.

Try this out. First, exit if !propertyValue. Then do the following to convert it to a PerformanceAttributeItemState.

const setPerformanceStateNode = async (propertyValue: unknown) => {
  if (!propertyValue) {
    return
  }
  
  const node = {
    id: String(propertyValue.id ?? ''),
    label: String(propertyValue.label ?? '')
  } as PerformanceAttributeItemState
    
    // the rest of the function. Do not need 'if (!node)` here.
}

Also in the code you display, part of Grafana's code, str should be a string when passed into quoted, but it looks like a PerformanceAttributeItemState instead (an id and label), so most likely something else is wrong.

[sbernabe1]

@synqotik, the following is the output when trying to test the updated code provided:

`const setPerformanceStateNode = async (propertyValue: unknown) => {
console.log('setPerformanceStateNode, propertyValue:')
console.dir(propertyValue)
  if (!propertyValue) {
    return
  }
    const node = {
            id: String(propertyValue.id ?? ''),
            label: String(propertyValue.label ?? '')
    } as PerformanceAttributeItemState
  //const node = JSON.parse(JSON.stringify(propertyValue)) as PerformanceAttributeItemState

  const resourceOptions: OnmsResourceDto[] = await loadResourcesByNode(node.id || node.label)
  const existingLabel = performanceState?.resource?.label

  let resource = performanceState?.resource ?? {}

  if (!isTemplateVariable(resource)) {
      const resourceOption = (existingLabel && resourceOptions && resourceOptions.filter(r => r.label === existingLabel)?.[0]) || null

      if (resourceOption?.id && resourceOption?.label) {
        resource = {
          id: resourceOption.id ?? '',
          label: resourceOption.label ?? ''
        }
      }
  }

  const state = {
    ...performanceState,
    node,
    resource
  } as PerformanceAttributeState

  setPerformanceState(state)
}`

Error's from "rpm run watch":

[sbernabe1]

@synqotik, I've made some updates to fix the error while still using the suggestions you recommended.

const setPerformanceStateNode = async (propertyValue: unknown) => {
console.log('setPerformanceStateNode, propertyValue:')
console.dir(propertyValue)
  if (!propertyValue) {
    return
  }
    // Correcting Property error's.
const nodeAsItemState = propertyValue as PerformanceAttributeItemState

// suggested code.
const node = {
    id: String(nodeAsItemState.id ?? ''),
    label: String(nodeAsItemState.label ?? '')
    } as PerformanceAttributeItemState
  //const node = JSON.parse(JSON.stringify(propertyValue)) as PerformanceAttributeItemState`

Looks like this works, no need for JSON implementation:

[synqotik]

@sbernabe1 ahh ok, try this instead, it's just a Typescript issue, need to cast propertyValue: unknown to an any first:

      if (!propertyValue) {
        return
      }
      
      const propertyValueAny = propertyValue as any
      
      const node = {
        id: String(propertyValueAny.id ?? ''),
        label: String(propertyValueAny.label ?? '')
      } as PerformanceAttributeItemState

Hopefully this works!

[sbernabe1]

@synqotik, this works!

Code:

Result:

[synqotik]

Nice! Looks good. I think it was just that the id is being passed in as a number and the TypeError was because we were assuming it was a string. Thanks for your work on this :)

[synqotik]

@sbernabe1 you can go ahead and make that change and I'll approve.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[sbernabe1]

@synqotik, thank you for validating this with me! I really appreciate the support :) I have gone ahead and updated the changes.

[synqotik]

LGTM!

[synqotik]

I went ahead and merged this