You're logged in, except it's an error

Question

I've automated downloading of data from a 3rd party website. The process involves logging into the site, downloading the data, then logging out - lather, rinse, repeat for each client.

The code is in VBA using Access as the host and Selenium Basic to handle the browser details. Hate all you want, but I've implemented it as a WebDownloader interface with multiple implementations (one for each of the sites I have to download from). Plus, when the only tool you have is a stone, everything looks like a nail....

Occasionally we can't get logged into the system for one or more of a variety of reasons (like our AP department is very slow about paying bills and a site will shut us off...).

The following code handles the login process. Prior to this, the browser is opened and the website's login page is retrieved. Once I've gotten to this point, it's safe to assume that everything is ready for the login process to begin.

This works, but it's ugly, and my concern is that the normal, expected course of action goes through the error handler.

Private Sub IWebDownloader_LoginToWebPage()

  Const USER_NAME_FIELD_ID As String = "login"
  Const PASSWORD_FIELD_ID As String = "password"
  Const LOGIN_BUTTON_CLASS_ID As String = "btn-form"

  LogManager.Log TraceLevel, "ApptPlusWebDownloader.IWebDownloader_LoginWebPage"

  On Error GoTo ErrorHandler
  With this.Driver
    .FindElementById(USER_NAME_FIELD_ID).SendKeys this.UserID
    .FindElementById(PASSWORD_FIELD_ID).SendKeys this.Password
    .FindElementByClass(LOGIN_BUTTON_CLASS_ID).Click
  End With
  Dim noteElement As WebElement
  Set noteElement = this.Driver.FindElementByClass("note")
  'assumes we find a note which, normally, we shouldn't
  this.IsLoggedIn = False
  If InStr(1, noteElement.text, "disabled") Then
    LogManager.Log WarnLevel, "Login disabled --- ApptPlus message: " & noteElement.text
  Else
    LogManager.Log WarnLevel, "Login warning --- ApptPlus message: " & noteElement.text
  End If

CleanExit:
  Exit Sub

ErrorHandler:
  With Err
    If .Number = 7 Then
      this.IsLoggedIn = True
    Else
      LogManager.Log ErrorLevel, "ApptPlus Failed to login. UserID: " & this.UserID
      this.IsLoggedIn = False
    End If
  End With
  Resume CleanExit

End Sub

The only way to determine if there's a login issue is to look for the note element appearing on the login page - failure to find that element is expected, preferred and is a good thing.

I could resolve this by wrapping with an OERN like this:

On Error Resume Next
Set noteElement = this.Driver.FindElementByClass("note")
'handle the expected error which is really a good thing here
On Error GoTo ErrorHandler

Is that the best way to go about this, or does someone see something better I could do?

I'm also open to any other tips or suggestions anyone sees that could improve the code.

Mathieu Guindon · Accepted Answer · 2017-11-01 14:09:38Z

Abstract it away!

Not sure exactly which instruction means to throw error 7 (which is normally an out of memory error code), but I think something like this should work - basically the idea is to extract the piece of logic into its own self-contained function, so that the calling code doesn't need to worry about an expected error and can use a simple Boolean value instead:

Private Function IsLoginEnabled(Optional ByRef outNoteElement As WebElement) As Boolean
    On Error Resume Next
        Set outNoteElement = this.Driver.FindElementByClass("note")
        IsLoginEnabled = outNoteElement Is Nothing And Err.Number = 7
        Err.Clear
    On Error GoTo 0
End Function

The calling code's logic can now leverage a simple Boolean result:

this.IsLoggedIn = IsLoginEnabled(noteElement)

It can also see that noteElement only ever contanis a valid object reference when the function returns False, so you don't need to be dealing with a run-time error in order to determine whether you need to ~~inspect that noteElement object~~ give your AP department a call.

Thank you! Finally got back to doing some code and put this in place. Sometimes the walls are so transparent, you don't realize the box you're in. — FreeMan
– FreeMan, Commented Nov 27, 2017 at 14:48

Stack Exchange Network

You're logged in, except it's an error

1 Answer 1

You must log in to answer this question.

Hot Network Questions

You're logged in, except it's an error

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions