Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--failon command does not return status 1 #71

Closed
SFDX-Sam opened this issue Aug 10, 2023 · 26 comments · Fixed by #72 or #80
Closed

--failon command does not return status 1 #71

SFDX-Sam opened this issue Aug 10, 2023 · 26 comments · Fixed by #72 or #80
Assignees
Labels
bug Something isn't working priority Has High Priority

Comments

@SFDX-Sam
Copy link

SFDX-Sam commented Aug 10, 2023

Good evening! I've implemented flow scanner on both a gitlab pipeline and my local windows machine, and I cannot get the scanner to return a status code of 1 when using the --failon command.

I run the following command:
sf flow:scan --failon error --config "scanner-config.json"

And it returns:

Identified 1 flows to scan... Scan complete
== DML Inside Loop Flow ==
Flow type: AutoLaunchedFlow

ERROR APIVersion
Details: 56
Newer API components may cause older versions of Flows to start behaving incorrectly due to differences in the underlying 
mechanics. The Api Version has been available as an attribute on the Flow since API v50.0 and it is recommended to limit 
variation and to update them on a regular basis.

ERROR FlowName
Details: The name DML_Inside_Loop_Flow does not meet the regex convention ===58
Readability of a flow is very important. Setting a naming convention for the Flow Name will improve the 
findability/searchability and overall consistency. It is recommended to at least provide a domain and a short description of 
the actions undertaken in the flow, in example Service_OrderFulfillment.


=== A total of 2 errors have been found in 1 flows.`

But does not seem to throw a status code of 1 - I have tried reducing the loglevel down to warning but still no luck.

image

@SFDX-Sam
Copy link
Author

SFDX-Sam commented Aug 10, 2023

For reference - my gitlab pipeline looks like this:

####################################################
# The docker image the jobs initialize with.
# We use nodejs, so a node image makes sense.
# https://docs.gitlab.com/ee/ci/yaml/README.html#image
####################################################
image: "node:latest"

####################################################
# The sequential stages of this pipeline.
# Jobs within a stage run in parallel.
# https://docs.gitlab.com/ee/ci/yaml/README.html#stages
####################################################
stages:
  - metadata_scan
  - apex_scan

####################################################
# Metadata Scanning Phase
####################################################
metadata_scan:
  stage: metadata_scan
  allow_failure: false
  script:
    - install_salesforce_cli
    - install_lightning_flow_scanner
    - scan_metadata

  artifacts:
    paths:
      - metadata-scan-files/
    when: always


####################################################
# Helper Methods
####################################################

.sf_helpers: &sf_helpers |
  function install_salesforce_cli() {

    # Salesforce CLI Environment Variables
    # https://developer.salesforce.com/docs/atlas.en-us.sfdx_dev.meta/sfdx_dev/sfdx_dev_cli_env_variables.htm

    # By default, the CLI periodically checks for and installs updates.
    # Disable (false) this auto-update check to improve performance of CLI commands.
    export SF_AUTOUPDATE_DISABLE=false

    # Set to true if you want to use the generic UNIX keychain instead of the Linux libsecret library or macOS keychain.
    # Specify this variable when using the CLI with ssh or "headless" in a CI environment.
    export SF_USE_GENERIC_UNIX_KEYCHAIN=true

    # For force:package:version:create, disables automatic updates to the sfdx-project.json file.
    export SF_PROJECT_AUTOUPDATE_DISABLE_FOR_PACKAGE_VERSION_CREATE=true

    # Install Salesforce CLI
    npm install @salesforce/cli --global

    # Output CLI version and plug-in information
    sf update
    sf --version
    sf plugins --core

  }
  #Install the flow scanner plugin - it is not digitally signed so we need to echo y to accept the prompt.
  function install_lightning_flow_scanner() {
    echo 'y' | sf plugins:install lightning-flow-scanner
  }

  # Function to run the metadata scan on the current branch
  function scan_metadata() {
    local OUTPUT_DIRECTORY=metadata-scan-files
    mkdir -p $OUTPUT_DIRECTORY
    sf flow:scan --failon error --config "scanner-config.json" 

  }

@RubenHalman
Copy link
Contributor

RubenHalman commented Aug 10, 2023

thank you for reporting this. First of all, I think it would be -failon "error" instead of -failon error.(not sure if that makes a difference) I am wondering however, if you don't use failing it is error level by default as per my understanding. so in this case you don't need the argument, although it should work ofcoùrse. what are you trying to achieve with the failon argument specifically?

@SFDX-Sam
Copy link
Author

Hey @RubenHalman !

I think I just put it because the documentation didn't specify if it was required for throwing a non-zero status code or not.

It seems that even without the --failon flag, it still returns a status code of 1 (success).

I also tried -failon "error", but no dice, it did not give me any errors however, so I guess it is an accepted param.

@RubenHalman
Copy link
Contributor

I am sorry. Can you show the --JSON output?

@SFDX-Sam
Copy link
Author

Sure thing

{
  "status": 0,
  "result": {
    "summary": {
      "flowsNumber": 1,
      "errors": 2,
      "message": "A total of 2 errors have been found in 1 flows.",
      "errorLevelsDetails": {
        "error": 2
      }
    },
    "status": 1,
    "results": [
      {
        "flowName": "DML Inside Loop Flow",
        "ruleName": "APIVersion",
        "description": "Newer API components may cause older versions of Flows to start behaving incorrectly due to differences in the underlying mechanics. The Api Version has been available as an attribute on the Flow since API v50.0 and it is recommended to limit variation and to update them on a regular basis.",
        "severity": "error",
        "type": "flow",
        "details": "56"
      },
      {
        "flowName": "DML Inside Loop Flow",
        "ruleName": "FlowName",
        "description": "Readability of a flow is very important. Setting a naming convention for the Flow Name will improve the findability/searchability and overall consistency. It is recommended to at least provide a domain and a short description of the actions undertaken in the flow, in example Service_OrderFulfillment.",
        "severity": "error",
        "type": "flow",
        "details": "The name DML_Inside_Loop_Flow does not meet the regex convention ===58"
      }
    ]
  }
}

Interestingly, I can see the status here for each error, but it does not seem to exit the oclif process with the exit code - it seems possible based on a quick look here

@RubenHalman
Copy link
Contributor

RubenHalman commented Aug 11, 2023

I probably need some input from @nvuillam here. Before v2, the core module threw an error resulting in an exit code, but it malformed the json. So I believe we changed it so the json is valid so it can be used for automation, but this is not really my expertise and requires a further deep dive

@nvuillam
Copy link
Collaborator

@RubenHalman @SFDX-Sam sorry for the delay (vacations ^^), this issue should be solved in #72 :)

@SFDX-Sam
Copy link
Author

Thanks both!

@RubenHalman
Copy link
Contributor

Hi @SFDX-Sam . Sorry for the delay and thank you again for your feedback. @nvuillam fix is included in the latest version 2.12. Please let us know if this sufficiently addresses your needs!

@RubenHalman RubenHalman reopened this Aug 30, 2023
@jorgesolebur
Copy link

jorgesolebur commented Sep 7, 2023

Hello ! I am still having the same issue, either calling
sfdx flow scan -c .flow-scanner.json -d "./force-app/" --json --failon "error" OR
sfdx flow scan -c .flow-scanner.json -d "./force-app/" --json --failon error

I am also using the last version

sf plugins
lightning-flow-scanner 2.12.0

This is the response (it includes errors):

{
  "status": 0,
  "result": {
    "summary": {
      "flowsNumber": 27,
      "errors": 5,
      "message": "A total of 5 errors have been found in 27 flows.",
      "errorLevelsDetails": {
        "error": 172,
        "warning": 54
      }
    },
    "status": 1,
    "results": [
      {
        "flowName": "NCS Clone Action Plan Template",
        "ruleName": "MissingFaultPath",
        "description": "Sometimes a flow doesn’t perform an operation that you configured it to do. By default, the flow shows an error message to the user and emails the admin who created the flow. However, you can control that behavior.",
        "severity": "error",
        "type": "pattern",
        "details": {
          "name": "Get_Action_Plan_Template_Details",
          "type": "recordLookups"
        }
      },
      {
        "flowName": "NCS Clone Action Plan Template",
        "ruleName": "MissingFaultPath",
        "description": "Sometimes a flow doesn’t perform an operation that you configured it to do. By default, the flow shows an error message to the user and emails the admin who created the flow. However, you can control that behavior.",
        "severity": "error",
        "type": "pattern",
        "details": {
          "name": "GET_Action_Plan_Template_Task_Details",
          "type": "recordLookups"
        }
      },
      {
        "flowName": "NCS Opportunity After Save",
        "ruleName": "FlowDescription",
        "description": "Descriptions are useful for documentation purposes. It is recommended to provide information about where it is used and what it will do.",
        "severity": "error",
        "type": "flow",
        "details": "undefined"
      },
      {
        "flowName": "NCS Opportunity Before Save",
        "ruleName": "MissingNullHandler",
        "description": "If a Get Records operation does not find any data it will return null. Use a decision element on the operation result variable to validate that the result is not null.",
        "severity": "error",
        "type": "pattern",
        "details": {
          "name": "Get_Deal_Path_TCV_Matrix_Record",
          "type": "recordLookups"
        }
      },
      {
        "flowName": "NCS Opportunity Before Save",
        "ruleName": "FlowDescription",
        "description": "Descriptions are useful for documentation purposes. It is recommended to provide information about where it is used and what it will do.",
        "severity": "error",
        "type": "flow",
        "details": "undefined"
      }
    ]
  }
}

@nvuillam
Copy link
Collaborator

nvuillam commented Sep 7, 2023

Hmmm it was supposed to be solved :(
I'll look into it soon !

@jorgesolebur
Copy link

Thanks

@jorgesolebur
Copy link

No pressure but do you have any idea on when this will be solved ? We are forced to stop using this fantastic tool in our CI because of this error

@RubenHalman
Copy link
Contributor

RubenHalman commented Oct 4, 2023

@jorgesolebur Im sorry. I have don't understand how this parameter is supposed to be working. Im waiting for@nvuillam or someone to explain this to me so I can implement a fix!

@jorgesolebur
Copy link

@nvuillam Please do not hesitate to details on how this works! From my understand, this issue is somehow related to #65, because if there is at least an error in the JSON output summary then the command should fail (returning error code 1). If the JSON output summary called "errors" = 0, then the command should return error code 0.

@RubenHalman RubenHalman added the priority Has High Priority label Oct 5, 2023
@nvuillam
Copy link
Collaborator

I'll try to fix that this weekend:)

@RubenHalman
Copy link
Contributor

@nvuillam Thanks. I am confused how the error level should impact the exit status.

@nvuillam
Copy link
Collaborator

I think I found a fix :)

@RubenHalman RubenHalman self-assigned this Nov 7, 2023
@RubenHalman
Copy link
Contributor

RubenHalman commented Nov 7, 2023

My apologies for the delay on publishing these changes. It should now be resolved with the latest version thanks to Nicolas efforts. Thank you very much for your involvement @SFDX-Sam, @jorgesolebur @nvuillam

@jorgesolebur
Copy link

Thanks! Let me test that! I will be very happy to be able to enable again this automation in my pipeline :-) will keep you posted!

@nvuillam
Copy link
Collaborator

@jorgesolebur I integrated Flow Scanner within MegaLinter (it is still in the beta version) , and it requires exit code > 0 in case of errors, so it should work on your side too :)

@jorgesolebur
Copy link

jorgesolebur commented Nov 15, 2023

@nvuillam Maybe I am missing something, but I am trying to execute the following command:
sf flow scan -c .flow-scanner.json -d "./force-app-delta/" --json --failon error

Which would mean that I only want the above command to return status 1 if they find any violation defined with a severity 'error'.

I have defined in my config json file all rules with a severity warning:

"rules":   {
    "APIVersion": {
      "severity": "warning",
      "expression": "===59"
    },
    "CopyAPIName": {
      "severity": "warning"
    },
    "DMLStatementInLoop": {
      "severity": "warning"
    },
    "DuplicateDMLOperation": {
      "severity": "warning"
    },
    "FlowDescription": {
      "severity": "warning"
    },
    "HardcodedId": {
      "severity": "warning"
    },
    "MissingFaultPath": {
      "severity": "warning"
    },
    "MissingNullHandler": {
      "severity": "warning"
    },
    "UnconnectedElement": {
      "severity": "warning"
    },
    "UnusedVariable": {
      "severity": "warning"
    }
  }

When executing the above command, it is returning a status code 1. This is not consistent because the scan found 2 violations that had a severity warning. The scan did not find any violation with severity = error:

{
  "status": 1,
  "result": {
    "summary": {
      "flowsNumber": 1,
      "results": 2,
      "message": "A total of 2 results have been found in 1 flows.",
      "errorLevelsDetails": {}
    },
    "status": 1,
    "results": [
      {
        "violation": {
          "metaType": "attribute",
          "name": "58",
          "subtype": "apiVersion",
          "expression": "===59"
        },
        "name": "58",
        "metaType": "attribute",
        "type": "apiVersion",
        "details": {
          "expression": "===59"
        },
        "ruleDescription": "Introducing newer API components may lead to unexpected issues with older versions of Flows, as they might not align with the underlying mechanics. Starting from API version 50.0, the 'Api Version' attribute has been readily available on the Flow Object. To ensure smooth operation and reduce discrepancies between API versions, it is strongly advised to regularly update and maintain them.",
        "rule": "Outdated API Version",
        "flowName": "NCS Approval Assignment Matrix Header Validation Before Save",
        "flowType": "AutoLaunchedFlow",
        "severity": "warning"
      },
      {
        "violation": {
          "element": {
            "name": [
              "Get_Existing_Approval_Assignment_Matrix_Header"
            ],
            "label": [
              "Get Existing Approval Assignment Matrix Header"
            ],
            "locationX": [
              "182"
            ],
            "locationY": [
              "395"
            ],
            "assignNullValuesIfNoRecordsFound": [
              "false"
            ],
            "connector": [
              {
                "targetReference": [
                  "Get_Existing_Approval_Assignment_Matrix_Header_Null_Check"
                ]
              }
            ],
            "filterLogic": [
              "1 AND 2 AND 3 AND ((4 AND 5) OR (6 AND 7) OR (9 AND 10)) AND 8"
            ],
            "filters": [
              {
                "field": [
                  "NCS_Country__c"
                ],
                "operator": [
                  "EqualTo"
                ],
                "value": [
                  {
                    "elementReference": [
                      "$Record.NCS_Country__c"
                    ]
                  }
                ]
              },
              {
                "field": [
                  "NCS_Territory__c"
                ],
                "operator": [
                  "EqualTo"
                ],
                "value": [
                  {
                    "elementReference": [
                      "$Record.NCS_Territory__c"
                    ]
                  }
                ]
              },
              {
                "field": [
                  "NCS_CSU__c"
                ],
                "operator": [
                  "EqualTo"
                ],
                "value": [
                  {
                    "elementReference": [
                      "$Record.NCS_CSU__c"
                    ]
                  }
                ]
              },
              {
                "field": [
                  "NCS_Start_Date__c"
                ],
                "operator": [
                  "GreaterThan"
                ],
                "value": [
                  {
                    "elementReference": [
                      "$Record.NCS_Start_Date__c"
                    ]
                  }
                ]
              },
              {
                "field": [
                  "NCS_Start_Date__c"
                ],
                "operator": [
                  "LessThanOrEqualTo"
                ],
                "value": [
                  {
                    "elementReference": [
                      "$Record.NCS_End_Date__c"
                    ]
                  }
                ]
              },
              {
                "field": [
                  "NCS_End_Date__c"
                ],
                "operator": [
                  "GreaterThan"
                ],
                "value": [
                  {
                    "elementReference": [
                      "$Record.NCS_Start_Date__c"
                    ]
                  }
                ]
              },
              {
                "field": [
                  "NCS_End_Date__c"
                ],
                "operator": [
                  "LessThanOrEqualTo"
                ],
                "value": [
                  {
                    "elementReference": [
                      "$Record.NCS_End_Date__c"
                    ]
                  }
                ]
              },
              {
                "field": [
                  "RecordTypeId"
                ],
                "operator": [
                  "EqualTo"
                ],
                "value": [
                  {
                    "elementReference": [
                      "$Record.RecordTypeId"
                    ]
                  }
                ]
              },
              {
                "field": [
                  "NCS_Start_Date__c"
                ],
                "operator": [
                  "LessThanOrEqualTo"
                ],
                "value": [
                  {
                    "elementReference": [
                      "$Record.NCS_Start_Date__c"
                    ]
                  }
                ]
              },
              {
                "field": [
                  "NCS_End_Date__c"
                ],
                "operator": [
                  "GreaterThanOrEqualTo"
                ],
                "value": [
                  {
                    "elementReference": [
                      "$Record.NCS_End_Date__c"
                    ]
                  }
                ]
              }
            ],
            "getFirstRecordOnly": [
              "true"
            ],
            "object": [
              "NCS_Approval_Assignment_Matrix_Header__c"
            ],
            "storeOutputAutomatically": [
              "true"
            ]
          },
          "subtype": "recordLookups",
          "metaType": "node",
          "connectors": [
            {
              "element": [
                {
                  "targetReference": [
                    "Get_Existing_Approval_Assignment_Matrix_Header_Null_Check"
                  ]
                }
              ],
              "processed": false,
              "type": "connector",
              "reference": "Get_Existing_Approval_Assignment_Matrix_Header_Null_Check"
            }
          ],
          "name": "Get_Existing_Approval_Assignment_Matrix_Header",
          "locationX": "182",
          "locationY": "395"
        },
        "name": "Get_Existing_Approval_Assignment_Matrix_Header",
        "metaType": "node",
        "type": "recordLookups",
        "details": {
          "locationX": "182",
          "locationY": "395",
          "connectsTo": [
            "Get_Existing_Approval_Assignment_Matrix_Header_Null_Check"
          ]
        },
        "ruleDescription": "At times, a flow may fail to execute a configured operation as intended. By default, the flow displays an error message to the user and notifies the admin who created the flow via email. However, you can customize this behavior by incorporating a Fault Path.",
        "rule": "Missing Fault Path",
        "flowName": "NCS Approval Assignment Matrix Header Validation Before Save",
        "flowType": "AutoLaunchedFlow",
        "severity": "warning"
      }
    ]
  }
}

Does it make sense ? Shall I crate a new issue ?

@RubenHalman RubenHalman reopened this Nov 24, 2023
@RubenHalman
Copy link
Contributor

@jorgesolebur Thank you very much for your input and im sorry for my limited involvement in this matter. I believe I have now resolved this issue with 2.16, but if you would be so kind to have a look I would greatly appreciate it.

@jorgesolebur
Copy link

Thanks! Will test it out soon

@jorgesolebur
Copy link

It is working now, thanks !
However I found another issue and will be created a ticket separately.

@RubenHalman
Copy link
Contributor

@jorgesolebur thank you so much for your patience and involvement. we will have a look at that ticket ASAP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment