snowflakedb / snowflake-connector-net

Snowflake Connector for .NET
Apache License 2.0
178 stars 138 forks source link

Externalbrowser authenticator failing for net6.0 #592

Closed davidhcar closed 1 year ago

davidhcar commented 1 year ago

Issue description

Regex match fails to differntiate net6.0 and net471 - The constant value defined in the Snowflake.Data.csproj that matches .Net full framework is matching both .net core and .net full framework as they follow same naming convention. This is causing for .net6 clients to execute/run net471 code for ExternalBrowserAuthentication(Snowflake.Data.Core.Authenticator.ExternalBrowserAuthenticator) which is failing to launch browser

Fix identified PR will be coming soon.

Example code

Snowflake.Data.csproj, line#3 <TargetFrameworks>net6.0;net472</TargetFrameworks>

line #20-22,

  <PropertyGroup Condition="$([System.Text.RegularExpressions.Regex]::IsMatch('$(TargetFramework)', '^net\d'))">
    <DefineConstants>NETFRAMEWORK</DefineConstants>
  </PropertyGroup>

Error log

{"$type":"Snowflake.Data.Client.SnowflakeDbException, Snowflake.Data","SqlState":"08006","QueryId":null,"ErrorCode":270001,"IsTransient":false,"BatchCommand":null,"Message":"Error: Snowflake Internal Error: Unable to connect. An error occurred trying to start process {SSO URL}

Configuration

Driver version: net6 VS 2022

davidhcar commented 1 year ago

I have a fix in place, it looks like I need to gain access to create PR, would like to know what would be the process to contribute to this code base.

colingreen-payroc commented 1 year ago

I believe the entire ProperyGroup can be removed, because NETFRAMEWORK is a standard preprocessor symbol when compiling for a .NET Framework target, and is not set when compiling for a .NET [Core] target.

davidhcar commented 1 year ago

I believe the entire ProperyGroup can be removed, because NETFRAMEWORK is a standard preprocessor symbol when compiling for a .NET Framework target, and is not set when compiling for a .NET [Core] target.

There is a conditional statement using this constant NETFRAMEWORK, so I dont think it will be removed until unless that part of the code is updated. Here is where this constant used, Snowflake.Data.Core.Authenticator.ExternalBrowserAuthenticator line https://github.com/snowflakedb/snowflake-connector-net/pull/175. My proposed change would not impact for folks using .net full framework, and .net core. I have seen some issues like that here, where the fix does not consider both type of frameworks.

colingreen-payroc commented 1 year ago

@davidhcar This should be resolved with the release of 2.0.23.

codewisdom commented 1 year ago

I'm running with 2.0.23 and it is still not working - it never launches the browser for me with .NET Framework or .NET 6. Can you validate the behavior locally? It's very easy to reproduce.

sfc-gh-dszmolka commented 1 year ago

thank you for raising this issue - we'll take a look

sfc-gh-dszmolka commented 1 year ago

thank you for the patience on this one. we couldn't reproduce the issue on 2.0.23 either. Setup: .NET 6.0 and .NET Framework 4.7.2 on Windows 11 and it works as expected.

could you please upgrade to the latest driver (2.0.25 today) and re-test ? if it still doesn't work, please share the

closing the issue for now but can continue working on this one with the above input for debugging.