Ability to set hints to fail_json and Fail-Json

[dagwieers]

Proposal: Ability to set hints to fail_json and Fail-Json

Author: Dag Wieers <@dagwieers>

Date: 2017-07-06

• Status: New
• Proposal type: core design
• Targeted Release: 2.4
• Associated PR: <link to GH PR in ansible/proposals if PR was submitted>
• Estimated time to implement: 1 hour

Motivation

Very specific use-case. A common user error in Windows is the use of backslashes for paths and how Ansible handles these in YAML and as key=value syntax.

So we added a test to check if the provided path is a valid path or not (it doesn't include any escape sequences by accident, like \t or \v). So that the output to the user goes from:

fatal: [computer61]: FAILED! => {
    "changed": false, 
    "failed": true, 
    "module_stderr": "An error occurred while creating the pipeline.\r\n    + CategoryInfo          : NotSpecified: (:) [], ParentContainsErrorRecordException\r\n    + FullyQualifiedErrorId : RuntimeException\r\n \r\nException calling \"Run\" with \"1\" argument(s): \"Exception calling \"Invoke\" with \"0\" argument(s): \"The running command st\r\nopped because the preference variable \"ErrorActionPreference\" or common parameter is set to Stop: Ongeldige tekens in p\r\nad.\"\"\r\nAt line:47 char:5\r\n+     $output = $entrypoint.Run($payload)\r\n+     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\r\n    + CategoryInfo          : NotSpecified: (:) [], ParentContainsErrorRecordException\r\n    + FullyQualifiedErrorId : ScriptMethodRuntimeException\r\n \r\n", 
    "module_stdout": "", 
    "msg": "MODULE FAILURE", 
    "rc": 1
}

to:

fatal: [computer61]: FAILED! => {
    "changed": false, 
    "failed": true, 
    "msg": "Get-AnsibleParam: Parameter 'path' has an invalid path 'C:\\Windows\temp' specified."
}

We want to be able to extend this output to something more helpful to users:

fatal: [computer61]: FAILED! => {
    "changed": false, 
    "failed": true, 
    "msg": "Get-AnsibleParam: Parameter 'path' has an invalid path 'C:\\Windows\temp' specified.",
    "hint": "Beware that you may have special characters included like \t or \v in your path."
}

The ability to add a hint to error messages is something that everybody should be using more often, and by splitting the message (fact) from the hint (possible resolutions) we make it much more convenient for the user than the common practice today of adding some hints to the error message.

If developers start using this more often in their code, they will also start to think about the possible failure cases and potential hints they have to provide. So we are less likely to get catchall exception-handling as a result and happier users !

Problems

What problems exist that this proposal will solve?

• Error messages often don't give hints to resolution
• Error messages that do include hints, may mix facts to possible resolutions, clearly identifying hints as such helps the user
• Adding this functionality could help in improving error output and user troubleshooting

Solution proposal

• We add a new (optional) keyword "hint" to the result object (on failure) as a convention
• We extend fail_json() and Fail-Json
  • Python

    fail_json(msg="Error message", hint="Possible resolution")

  • Powershell

    Fail-Json -obj $resultobj `
           -message "Get-AnsibleParam: Parameter '$name' has an invalid path '$value' specified." `
           -hint "Beware that you may have special characters included like \t or \v."

• We add the parameter hint: to the fail module
• We document this in the developer guide
• (Optionally) We add a flag to ansible configuration display_hints so users can disable it

[bcoca]

The 'hints' should already be present in the msg field, this is part of what we review for in modules.

Formalizing this as different field would make it 'required' ... which is a problem with existing code.

[dagwieers]

Why would formalizing hint in a different field make it required ? It would just be a recommendation to do it like this in the future.

[jborean93]

I agree with @bcoca the actual error message should contain the hint of the error (or we add the error ourselves alongside the exception) about what we think it could be. Having another field to but this would just complicate something that I believe is quite simple to do.

[bcoca]

@dagwieers i view it as 'requried' part of your msg now, by making it an additional field it makes it seem optional, which I think leads to worse error messages, not better.

[dagwieers]

@bcoca Most of the error message do not include a hint today. So it is optional now anyway.