The Execute Method Issue

[GCuser99]

Hi, here are some thoughts for improving the function and clarity of the Execute method code logic. The Execute method handles the processing of the immediate inputs and outputs to/from the Selenium WebDriver and as such, is a centrally important code.

The section of code from Execute that I would like to discuss here is the complex logic in Execute's processing of the return response from Selenium WebDriver, as seen below.

 'Return value(s)
    If IsNull(resp("value")) Then
        Set Execute = New Dictionary
    ElseIf TypeName(resp("value")) = "Collection" Then
        Set Execute = resp("value")
    ElseIf VarType(resp("value")) = vbObject Then
        If resp("value").Exists("error") Then
            Err.Raise 513, "WebDriver.Execute", JsonConverter.ConvertToJson(resp("value"))
        Else
            Set Execute = resp("value")
        End If
    Else
        Execute = resp("value")
    End If

One of the very nice aspects of the response that is directly received from the Selenium WebDriver is that it always comes returned as a Dictionary object containing different types (collections, arrays, strings, etc) depending on the command sent, including error information. That information can be parsed by the caller routines given the caller's expectations (the context). Execute shouldn't have to worry at all about preparing or (partially) parsing information for the caller. With that in mind, I am proposing that Execute should pass the response dictionary in its raw form and then let the caller decide how to parse, as in this below:

Public Function Execute(driverCommand, Optional parameters As Dictionary = Nothing, Optional ByVal raise As Boolean = True) As Dictionary
    Dim method As String, path As String
    Dim cmdArgs As New Dictionary, parmKey As Variant
    Dim response As Dictionary
    
    method = driverCommand(0)
    path = driverCommand(1)
    
    If parameters Is Nothing Then
        Set parameters = New Dictionary
    End If
    
    'Set default session id if session id is missing
    If Not parameters.Exists("sessionId") Then
        parameters.Add "sessionId", DefaultSessionId
    End If
    
    For Each parmKey In parameters
        If InStr(path, "$" & parmKey) > 0 Then 'path parameter
            path = Replace(path, "$" & parmKey, parameters(parmKey))
        Else 'non-path parameter
            cmdArgs.Add parmKey, parameters(parmKey)
        End If
    Next parmKey
    
    'sessionId should never be passed as an argument, but it will if $sessionId is not in path
    'see CMD_STATUS, CMD_NEW_SESSION, CMD_GET_ALL_SESSIONS
    If cmdArgs.Exists("sessionId") Then cmdArgs.Remove "sessionId"
    
    'Send request to selenium server
    Set response = SendRequest(method, UrlBase + path, cmdArgs)
    
    If raise And IsResponseError(response) Then 'trap error if occurs
        Err.raise 513, "WebDriver.Execute", GetResponseErrorMessage(response)
    End If

    Set Execute = response 'always pass a dictionary object - let caller parse info based on context, including error

End Function

Private Function GetResponseErrorMessage(resp As Dictionary) As String
    GetResponseErrorMessage = ""
    If TypeName(resp("value")) = "Dictionary" Then
        If resp("value").Exists("error") Then
            GetResponseErrorMessage = JsonConverter.ConvertToJson(resp("value"))
        End If
    End If
End Function

Private Function IsResponseError(resp As Dictionary) As Boolean
    IsResponseError = False
    If TypeName(resp("value")) = "Dictionary" Then
        If resp("value").Exists("error") Then
            IsResponseError = True
        End If
    End If
End Function

Note that the only task this simplified version of Execute is being asked to do with the response is to serve as the central error trapping mechanism, if the caller decides not to handle errors (raise=true). The two helper functions can be reused by the caller if needed, to handle unraised errors. A few examples of callers handling the errors:

Public Function Navigate(ByVal url As String, Optional ByVal SessionId As String = vbNullString, Optional ByVal timeOutms, Optional ByVal raise As Boolean = True) As Boolean
    Dim Data As New Dictionary, resp As Object, savtimeOutms,
    
    If SessionId <> vbNullString Then
        Data.Add "sessionId", SessionId
    End If
    
    Data.Add "url", url
    If Not IsMissing(timeOutms) Then
        savtimeOutms = Me.GetPageLoadTimeout(SessionId)
        If savtimeOutms <> timeOutms Then Me.SetPageLoadTimeout timeOutms, SessionId
    End If
    
    Set resp = Execute(CMD_GET, Data, raise)

    If Not IsMissing(timeOutms) Then
        If savtimeOutms <> timeOutms Then Me.SetPageLoadTimeout savtimeOutms, SessionId
    End If
    
    If IsResponseError(resp) Then
        Navigate = False
        Debug.Print GetResponseErrorMessage(resp)
    Else
        Navigate = True
    End If
    
End Function

Public Function IsAlertPresent(Optional ByVal SessionId As String = vbNullString) As Boolean
    Dim Data As New Dictionary, resp As Dictionary
    If SessionId <> vbNullString Then
        Data.Add "sessionId", SessionId
    End If

    Set resp = Execute(CMD_W3C_GET_ALERT_TEXT, Data, False) 'don't raise an error if not present
    
    If IsResponseError(resp) Then
        IsAlertPresent = False
    Else
        IsAlertPresent = True
    End If
End Function

Public Function IsElementPresent(by_ As by, ByVal value As String, Optional ByVal SessionId As String = vbNullString) As Boolean
    Dim timeout As Double, resp As Dictionary, Data As Dictionary
    
    timeout = GetImplicitlyWait(SessionId)
    If timeout <> 0 Then Me.SetImplicitlyWait 0, SessionId '' ImplicitlyWait Disable
   
    Set Data = ToSelector(by_, value)
    If SessionId = vbNullString Then
        Data.Add "sessionId", DefaultSessionId
    Else
        Data.Add "sessionId", SessionId
    End If
    
    Set resp = Execute(CMD_FIND_ELEMENT, Data, False) 'don't raise an error if not present

    If IsResponseError(resp) Then
        IsElementPresent = False
    Else
        IsElementPresent = True
    End If
    
    If timeout <> 0 Then Me.SetImplicitlyWait timeout, SessionId

End Function

Public Function ExecuteScript(ByVal Script As String, Optional ScriptArgs As Variant = vbNullString, Optional element As WebElement, Optional ByVal SessionId As String = vbNullString, Optional ByVal timeOutms, Optional ByVal raise As Boolean = True)
    Dim Data As New Dictionary, ElmData As New Dictionary, args As Variant

    Data.Add "script", Script
    
    ' Set ElementID and Args
    If Not element Is Nothing Then
        ElmData.Add "ELEMENT", element.elementid
        ElmData.Add ELEMENT_KEY, element.elementid
        args = Array(ElmData)
        If VarType(ScriptArgs) = vbVariant + vbArray Then    'Scriptargs is Array
            Dim i As Integer
            For i = 0 To UBound(ScriptArgs)
                ReDim Preserve args(i + 1)
                args(i + 1) = ScriptArgs(i)
            Next i
        Else
            args = Array(ElmData, ScriptArgs)
        End If
    Else
        args = Array()
    End If
    Data.Add "args", args
    
    ' Set sessionId
    If SessionId = vbNullString Then
        Data.Add "sessionId", DefaultSessionId
    Else
        Data.Add "sessionId", SessionId
    End If
    
    If Not IsMissing(timeOutms) Then
        savtimeOutms = Me.GetScriptTimeout(SessionId)
        If savtimeOutms <> timeOutms Then Me.SetScriptTimeout timeOutms, SessionId
    End If
    
    Dim resp As Dictionary
    
    Set resp = Execute(CMD_W3C_EXECUTE_SCRIPT, Data, raise)
    
    If IsResponseError(resp) Then
        ExecuteScript = GetResponseErrorMessage(resp)
        'ExecuteScript = resp("value")("error")
    Else
        ExecuteScript = resp("value")
    End If
    
    If Not IsMissing(timeOutms) Then
        If savtimeOutms <> timeOutms Then Me.SetScriptTimeout savtimeOutms, SessionId
    End If
    
End Function

The additional coding effort needed to update the callers for this change is close to trivial. All that has to be done is search the WebDriver class for this pattern "=Execute(CMD_" and where found, append a ("value") on the end of the Execute() call as in:

Public Function GetValue(element As WebElement, Optional ByVal SessionId As String = vbNullString) As String
    Dim Data As New Dictionary
    If SessionId <> vbNullString Then
        Data.Add "sessionId", SessionId
    End If
    Data.Add "id", element.elementid
    Data.Add "name", "value"
    GetValue = Execute(CMD_GET_ELEMENT_ATTRIBUTE, Data)("value") 'GCUser99 added ("value")
End Function

In a case like below, where the caller is indifferent about the response, no changes have to be made:

Public Sub Click(element As WebElement, Optional ByVal SessionId As String = vbNullString)
    Dim Data As New Dictionary
    If SessionId <> vbNullString Then
        Data.Add "sessionId", SessionId
    End If
    Data.Add "id", element.elementid
    Execute CMD_CLICK_ELEMENT, Data
End Sub

That is all that has to be done to simplify Execute. I would be interested in hearing what you think about this suggestion, and I would be willing to share my WebDriver class if that would be helpful (as a newbie to GitHub, I don't yet know how best to do that).

Thanks again for such an excellent resource.

[GCuser99]

For an example of same implementation of Execute as I'm proposing above see:

https://github.com/SeleniumHQ/selenium/blob/trunk/py/selenium/webdriver/remote/webdriver.py