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

Bug: When using connection strings which do not require UIDs, export fails #420

Closed
hecon5 opened this issue Aug 3, 2023 · 17 comments
Closed
Labels
pending resolved Possibly resolved, needs testing or confirmation

Comments

@hecon5
Copy link
Contributor

hecon5 commented Aug 3, 2023

When exporting SQL data, it will fail here; the connection string is the same as is used in our DB, so I know it's good, but we don't use a UID or Password in our environment (we use Windows / AD Auth).

image

image

@joyfullservice
Copy link
Owner

Hmm... Interesting. Most of our connections also use integrated authentication and didn't have any problem with this line. The ID and Password parameters are optional strings, so passing an empty string seemed to work fine in my testing. If you comment out those two parameters, does the connection open successfully?

Another thing to test would be to add a linked table to the desired SQL server (if you don't have one already) and then select the connection string from the dropdown list in the external database configuration. This would give you the same connection string used for the linked table, which seemed to work well in my environment.

Let me know what you find out!

@bclothier
Copy link
Contributor

FWIW, I vaguely recall running into this before - even though in theory a empty string would indicate so, it's not really equivalent to omitting the parameters and I end up having a conditional like so:

If Len(UserName) Then
  con.Open ConnectionString, UserName, Password
Else
  con.Open ConnectionString
End If

@joyfullservice
Copy link
Owner

FWIW, I vaguely recall running into this before - even though in theory a empty string would indicate so, it's not really equivalent to omitting the parameters and I end up having a conditional like so:

If Len(UserName) Then
  con.Open ConnectionString, UserName, Password
Else
  con.Open ConnectionString
End If

Thanks! I was wondering about that... It's kind of hard to tell how they are implementing the parameter check. Some things seem to check for a missing parameter, while others test the value itself.

@hecon5
Copy link
Contributor Author

hecon5 commented Aug 3, 2023

If you comment out those two parameters, does the connection open successfully?

Unfortunately... no :(

Another thing to test would be to add a linked table to the desired SQL server (if you don't have one already) and then select the connection string from the dropdown list in the external database configuration.

That was my first thought; used the drop down (handy feature, BTW!); I did confirm the table did have data and a connection, and it worked fine (to the linked table).

even though in theory a empty string would indicate so, it's not really equivalent to omitting the parameters and I end up having a conditional like so:

Ditto, we don't pass username or password in our setup because we ran into the same issue.

@bclothier
Copy link
Contributor

ADO library happens to contain lot of delightful surprises; it's super incredibly helpful how they silently substitute invalid combinations with something that you didn't ask for and then give you an cryptic "multi-step operation failed" error. :)

I think Clippy may have a hand in this.

@bclothier
Copy link
Contributor

If you comment out those two parameters, does the connection open successfully?

Unfortunately... no :(

Do you explicitly specify Trusted_Connection or Integrated Security in your connection string? Does it make a difference?

@hecon5
Copy link
Contributor Author

hecon5 commented Aug 3, 2023

I think Clippy may have a hand in this.

I think so...it's really the only explanation! We spent so much time banging our head on some hard rocks trying to figure out why some calls work just fine, but others, using the same syntax and connection won't, only to discover some arcane switch or situation which is buried deep in some random forum.

@hecon5
Copy link
Contributor Author

hecon5 commented Aug 3, 2023

Do you explicitly specify Trusted_Connection or Integrated Security in your connection string? Does it make a difference?

Yes, I even tried (shudder) to disable all encryption and integrated security as variants...and it doesn't make a difference.

joyfullservice added a commit that referenced this issue Aug 3, 2023
Gracefully logs errors if connection to database fails. #420
@hecon5
Copy link
Contributor Author

hecon5 commented Aug 3, 2023

OK! Getting somewhere!

So, turns out, that I'd forgotten with our application (probably blocked out the bad memories), despite declaring the Provider=MSOLEDBSQL19, when the connection gets formed, something in the background (pretty sure it's not our code...it just happens, but I'm going to dig more into this to verify) alters the provider to MSOLEDBSQL.1. I don't know exactly WHY this is, but it's what happens, even if we specify the connection string as MSOLEDBSQL19, it will automatically revert to MSOLEDBSQL.1 when the connection is actually opened.

I revised the connection string to manually use MSOLEDBSQL.1, and it works!

I'm going to troubleshoot to see why the Addin won't work like that, and update here with comments and a potential fix, and a PR to provide an example fix.

@bclothier
Copy link
Contributor

Hmm. Are you aware that the OLEDB 19 changed how it behaves compared to OLEDB 18? I'm not sure MSOLEDBSQL.1 is a valid string and thinking it's falling back to OLEDB 18.

See this article for details: https://techcommunity.microsoft.com/t5/sql-server-blog/ole-db-driver-19-0-for-sql-server-released/ba-p/3170362

I assume that you are using a local SQL Server in which case, you would have to use Trust Server Certificate or disable encryption in order to connect to a SQL Server that is using a self-signed certificate (the default behavior when installing).

@hecon5
Copy link
Contributor Author

hecon5 commented Aug 3, 2023

I'm not sure MSOLEDBSQL.1 is a valid string and thinking it's falling back to OLEDB 18.

We thought so, too, but when we print the connectionstring from an open connection, it spits out MSOLEDBSQL.1.

I am also trying to remember why we couldn't use the ODBC connection string which all our linked tables use to connect; It's been so long since we built those bits I can't remember.

@bclothier
Copy link
Contributor

I just confirmed - MSOLEDBSQL.1 is in fact OLEDB 18, which also explains the difference in the behavior from MSOLEDBSQL19 (which incidentally changes to MSOLEDBSQL19.1) as per the linked article.

Just to note - ODBC 18 has the same behavior change as OLEDB 19. ODBC 17 is same as OLEDB 18. Don't you love Microsoft's versioning!

@hecon5
Copy link
Contributor Author

hecon5 commented Aug 3, 2023

Yes...this has caused so much weeping and gnashing of teeth over the years of using this.
I also just confirmed that the drivers we thought were installed (named Version19 in the install package) are actually version 18, which makes more sense now, too. So I'm going to install the "right" version again, and see if that works.

One thing we did for our connection string validator was to have it try multiple attempts at connecting, and various drivers. This allows us to have driver updates rolled out and not break the thing for users who haven't been updated yet, but preferentially use the latest driver. I'll share that method with the PR when I get to the bottom of this.

@mwolfe02
Copy link

mwolfe02 commented Aug 3, 2023

Don't you love Microsoft's versioning!

I think they use "some antics" versioning.

@hecon5
Copy link
Contributor Author

hecon5 commented Aug 3, 2023

Ok, so here's a hacked up variant of what we use to convert ODBC connection strings to OLE connection strings. This allows us to only drag around one connection string per server, and allow for more than one driver package and still function. It allows us to rapidly parse and attempt connection strings to adapt to conditions. I don't know if this application should use it, but we've found it solved so many headaches with managing strings, because users don't know (or more importantly: care) if the driver is the latest or not, and saves devs from playing whackamole with drivers / config roulette.

Note, the Log we use has slightly different inputs to the addin, so they may need tweaked to be dropped in if they're used.

'---------------------------------------------------------------------------------------
' Procedure : GetParsedADOConnectionString
' Author    : hecon5
' Date      : 8/3/2023
' Purpose   : For various reasons, ADO connection strings have some oddities,
'           : and differing driver installations require differing strings. 
'           : This function will parse a Connection string based on some parameters and 
'           : attempt to provide a functional one.
'---------------------------------------------------------------------------------------
Public Function GetParsedADOConnectionString(ByRef strConnectionString As String _
                                            , Optional ByRef FailCount As Long = 0) As String
    Dim currConnString As String
    Dim ConnectionProperties() As String
    Dim PropertyCount As Long
    Dim strProperty As String

    Perf.OperationStart ModuleName & ".GetADOConnectionString"

    ConnectionProperties = Split(strConnectionString, ";")

    With New clsConcat
        .AppendOnAdd = ";"
        ' Add some beginning properties that are specific to ADO (OLE) connection strings.
            Select Case FailCount
                Case 0 To 1
                    .Add "Provider=MSOLEDBSQL19"
                    .Add "Connect Retry Count=3"

                Case 2
                    .Add "Provider=MSOLEDBSQL19"
                    .Add "Connect Retry Count=3"
                Case 3
                    .Add "Provider=MSOLEDBSQL"
                    .Add "Encrypt=yes"
                    .Add "Trusted_Connection=yes"
                    .Add "Connect Retry Count=3"
                Case 4
                    .Add "Provider=SQLNCLI11"
                    .Add "Trusted_Connection=yes"
                    .Add "Encrypt=yes"
                Case ADODBConnectRetryMaximum
                    .Add "Provider=SQLOLEDB"
                    .Add "Trusted_Connection=True"
                    .Add "Encrypt=yes"
                Case Else
                    .Add "Provider=SQLOLEDB"
                    .Add "Encrypt=true"
                    .Add "Trusted_Connection=True"
            End Select

        For PropertyCount = 0 To UBound(ConnectionProperties)
            strProperty = ConnectionProperties(PropertyCount)
            Select Case True ' This looks odd, yes, but it really just means we can use the comma delimited feature of "case"
            Case _
                Len(strProperty) < 1, Trusted_Connection
                StartsWith(strProperty, "Provider=", vbTextCompare), _
                StartsWith(strProperty, "ODBC", vbTextCompare), _
                StartsWith(strProperty, "DataTypeCompatibility=", vbTextCompare), _
                StartsWith(strProperty, "Driver=", vbTextCompare), _
                StartsWith(strProperty, "Trusted_Connection=", vbTextCompare), _
                StartsWith(strProperty, "Encrypt=", vbTextCompare)
                ' Don't include these; they aren't needed (or are specified elsewhere).

            Case _
                StartsWith(strProperty, "UID=", vbTextCompare), _
                StartsWith(strProperty, "PWD=", vbTextCompare)
                ' Do something? 
            Case Else
                .Add strProperty
            End Select
        Next PropertyCount
        
        ' These properties should go at the end.
        .Add "Use Encryption for Data=true"
        .Add "MARS Connection=True"
        .Add "Integrated Security=SSPI"
        .Add "DataTypeCompatibility=80"
        
        Log.AddEntry ModuleName & ".GetADOConnectionString", "ConnectionString: " & .GetStr, eelDebugInfo
    GetBackendADOConnectionString = .GetStr
    End With
    Perf.OperationEnd
End Function



'---------------------------------------------------------------------------------------
' Procedure : GetBackendConnectionADO
' Author    : hecon5
' Date      : 8/3/2023
' Purpose   : For various reasons, ADO connection strings have some oddities,
'           : and differing driver installations require differing strings. 
'           : This function will attempt to provide a connection given a connection string.
'---------------------------------------------------------------------------------------
Public Function GetBackendConnectionADO(ByRef strConnectionString As String) As ADODB.Connection
    
    Const FunctionName As String = ModuleName & ".GetBackendConnectionADO"
    Static FailCount As Long
    
    Dim ThisAttempt As Long
    Dim NewConnectionString As String
    
    Perf.OperationStart FunctionName
    On Error Resume Next
    
    ThisAttempt = FailCount ' Set the count for now; this allows us to test if the connection will fail.
    
    If m_ADOConnectionRemote Is Nothing Then
Rebuild_Connection:
        Perf.OperationStart FunctionName & ".Create"
        
        If Not BackendIsConnected Then GoTo Exit_Here
        
        Set m_ADOConnectionRemote = New ADODB.Connection
RetryConnection:
        With m_ADOConnectionRemote
            ' If the fail count is higher than 0 use the legacy
            NewConnectionString = GetBackendADOConnectionString(strConnectionString, FailCount)
            .ConnectionString = NewConnectionString
            .ConnectionTimeout = 60
            .CommandTimeout = 120
            .Open
            Pause 1
            If CatchAny(eelError, vbNullString, vbNullString, False, False) Or (.State And adStateOpen) <> adStateOpen Then
                 If ((FailCount - ThisAttempt) <= ADODBConnectRetryMaximum) Then
                ' If there's an error try again until maximum attempts and then tell user.
                    FailCount = FailCount + 1
                    
                    CatchAny eelDebugInfo, "Backend connection couldn't be opened. Retrying." & vbNewLine & vbNewLine & _
                                            "Attempted Connection String: " & NewConnectionString & vbNewLine & vbNewLine & _
                                            "Attempt: " & FailCount, FunctionName
    
                    GoTo RetryConnection
    
                Else
                    Log.Error eelCritical, "Backend Connection to the server could not be created. " & vbNewLine & vbNewLine & _
                                            "    You will not be able to use many of the database features. " & vbNewLine & vbNewLine & _
                                            " Please notifiy admins of the issue and include the log." _
                                            , FunctionName, , "Database connection Failed!" _
                                            , "Connection String: " & NewConnectionString & vbNewLine & vbNewLine & _
                                                "Attempt: " & FailCount
                End If
            End If
            
            CatchAny eelError, "Backend connection couldn't be opened." & _
                                vbNewLine & vbNewLine & _
                                "Connection String: " & NewConnectionString & vbNewLine & vbNewLine & _
                                "Attempt: " & FailCount, FunctionName
        End With
        Perf.OperationEnd
    End If
    
    If (m_ADOConnectionRemote.State And adStateOpen) <> adStateOpen Then m_ADOConnectionRemote.Open
    If Not ADOIsConnected(m_ADOConnectionRemote) Then GoTo Rebuild_Connection
    Set GetBackendConnectionADO = m_ADOConnectionRemote

    Log.AddEntry FunctionName, "Connection : " & vbNewLine & _
        m_ADOConnectionRemote.ConnectionString & vbNewLine & vbNewLine & _
        "State: " & m_ADOConnectionRemote.State, eelDebugInfo
    
    CatchAny eelError, "Backend connection couldn't be established." & vbNewLine & vbNewLine & _
                    "Connection String: " & m_ADOConnectionRemote.ConnectionString, FunctionName
Exit_Here:
    Perf.OperationEnd
End Function

@joyfullservice
Copy link
Owner

I think I am leaning towards just leaving the connection string responsibility with the user. Many (most?) might be able to select a connection string from the dropdown, but if something special is involved, we can let the devs decide exactly what they want to use for the connection string and enter it manually.

@joyfullservice joyfullservice added the pending resolved Possibly resolved, needs testing or confirmation label Aug 4, 2023
@hecon5
Copy link
Contributor Author

hecon5 commented Aug 7, 2023

Sounds good; I think that ultimately may be the best choice; fewer ghosts in the machine making people wonder why the string doesn't match what they expect.

@hecon5 hecon5 closed this as completed Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending resolved Possibly resolved, needs testing or confirmation
Projects
None yet
Development

No branches or pull requests

4 participants