Proper use of SetError in a function

[GPinzone]

I have a function that runs an external command and captures the command-line output. Depending on the text returned, I determine the error condition and return from the function with SetError.

expandcollapse popup

Func RunProgram($sInputFilename)

        Local $iPID = Run(@ComSpec & ' /C program.exe "' & $sInputFilename '" 2>&1', @ScriptDir, @SW_HIDE, $STDOUT_CHILD)

        ; Wait until the process has closed using the PID returned by Run.
        ProcessWaitClose($iPID)

        If @error Then
            LogError("It appears there was an error running the program: " & $sInputFilename)
            Return SetError(1, 3, "ProcessWaitClose error.")
        EndIf

        ; Read the Stdout stream of the PID returned by Run. This can also be done in a while loop. Look at the example for StderrRead.
        Local $sOutput = StdoutRead($iPID)

        ; Error reading from standard output.
        If @error Then
            LogError("Could not read output: " & $sInputFilename)
            Return SetError(1, 4, "Could not read stdout.")
        EndIf

        ; Strip whitespace from output.
        $sOutput = StringStripWS($sOutput, BitOR($STR_STRIPLEADING, $STR_STRIPTRAILING))

        ; Check for permission error.
        If StringInStr($sOutput, "Permission denied") Then
            LogError("Permission denied: " & $sInputFilename)
            Return SetError(1, 1, "Permission denied.")
        EndIf

        ; Unexpected output.
        If $sOutput <> "" Then
            LogError("Unexpected error: " & $sInputFilename & @CRLF & '"' & $sOutput & '"')
            Return SetError(1, 99, "Unexpected error.")
        EndIf
    EndIf

EndFunc

Func LogError($sText)
    MsgBox(BitOR($MB_SYSTEMMODAL, $MB_ICONERROR), "Error", $sText & @CRLF)
EndFunc   ;==>LogError

 

I'm not actually using any of the SetError information returned by the function in the main program.  A MsgBox gets displayed by the function itself.  Is this considered good practice?

 

Edited October 13, 2017 by GPinzone

[water]

ProcessWaitClose as you use it never sets @error. I suggest to check for errors after the preceding "Run" statement.

In addition i suggest to write the value of @error and @extended to your log file as well. Makes debugging much easier

StdoutRead sets @error when there is no data left to read. Do you always expect some data to be read from Stdout?

[mikell]

"Return" defines the value returned by the main function
Example :

Pseudo-code

$result = RunProgram($sInputFilename)
Msgbox(0,"", $result)

Func RunProgram($sInputFilename)
.....
If ... Return SetError(1, 4, "Error was here")
....
EndFunc

Real example (from help file)

#include <MsgBoxConstants.au3>

Local $fResult = myDiv(5, 0)
MsgBox($MB_SYSTEMMODAL, "Result", $fResult)
Exit

Func myDiv($iDividend, $iDivisor)
    If $iDivisor = 0 Then
        Return SetError(1, 0,"Plain division by zero")
    EndIf
    Return $iDividend / $iDivisor
EndFunc   ;==>myDiv

 

[GPinzone]

9 minutes ago, water said:

ProcessWaitClose as you use it never sets @error. I suggest to check for errors after the preceding "Run" statement.

In addition i suggest to write the value of @error and @extended to your log file as well. Makes debugging much easier

StdoutRead sets @error when there is no data left to read. Do you always expect some data to be read from Stdout?

According to https://www.autoitscript.com/autoit3/docs/functions/ProcessWaitClose.htm

it says "Failure:     0 if the wait timed out. On invalid PID the @error flag is set to non-zero and @extended is set to 0xCCCCCCCC."

If something returns an @error flag, I should check for it, yes?

I don't have a log file. The LogError function also displays the error message on a Log window on-screen, too, but I cut that out in the example.

[GPinzone]

14 minutes ago, mikell said:

"Return" defines the value returned by the main function
Example :

Pseudo-code

$result = RunProgram($sInputFilename)
Msgbox(0,"", $result)

Func RunProgram($sInputFilename)
.....
If ... Return SetError(1, 4, "Error was here")
....
EndFunc

Real example (from help file)

#include <MsgBoxConstants.au3>

Local $fResult = myDiv(5, 0)
MsgBox($MB_SYSTEMMODAL, "Result", $fResult)
Exit

Func myDiv($iDividend, $iDivisor)
    If $iDivisor = 0 Then
        Return SetError(1, 0,"Plain division by zero")
    EndIf
    Return $iDividend / $iDivisor
EndFunc   ;==>myDiv

 

Yes, the example makes sense since not only are we identifying an error, but we're returning a text string that goes into the result. (Nevermind that we're passing a string into variable that expects a float, but I digress...)

My program doesn't really use the values returned by SetError. I suppose they could one day, but they don't now. The Returns are really to control program flow. The function stops processing on an error thanks to the Return. It would work just as well using  "Return False." The main program doesn't care what it actually returns. All the error messages are generated by the function, where in your example from the help file, the return value is the error message and is handled by the main program.

I suppose I could eliminate the Returns altogether by using If Then..ElseIf..Else for the whole function. The function works as is, but I am more concerned about good practice.

[water]

10 minutes ago, GPinzone said:

According to https://www.autoitscript.com/autoit3/docs/functions/ProcessWaitClose.htm
it says "Failure:     0 if the wait timed out. On invalid PID the @error flag is set to non-zero and @extended is set to 0xCCCCCCCC."
If something returns an @error flag, I should check for it, yes?

My bad . I mixed up Return value and @error.

[mikell]

7 hours ago, GPinzone said:

My program doesn't really use the values returned by SetError

Hum, am I wrong or the topic title was "Proper use of SetError in a function" ?
If you need none of the values returned by SetError, then just don't use SetError and use instead the good old "if @error Then Return". This is good practice