NRE on Fake.Sql.SqlPackage

isaacabraham commented 2 years ago

Description

Calling deployDb causes an NRE.

Repro steps

Execute the following code in a script:

#r "nuget: Fake.Sql.SqlPackage"
Fake.Sql.SqlPackage.deployDb id

Expected behavior

System.Exception: No SqlPackage.exe filename was given.

Actual behavior

Binding session to 'C:/Users/IsaacAbraham/.nuget/packages/fake.sql.sqlpackage/5.20.4/lib/netstandard2.0/Fake.Sql.SqlPackage.dll'...
Binding session to 'C:/Users/IsaacAbraham/.nuget/packages/fake.core.process/5.20.4/lib/netstandard2.0/Fake.Core.Process.dll'...
Binding session to 'C:/Users/IsaacAbraham/.nuget/packages/fake.core.environment/5.20.4/lib/netstandard2.0/Fake.Core.Environment.dll'...
Binding session to 'C:/Users/IsaacAbraham/.nuget/packages/fake.io.filesystem/5.20.4/lib/netstandard2.0/Fake.IO.FileSystem.dll'...
System.NullReferenceException: Object reference not set to an instance of an object.
   at Fake.Sql.SqlPackage.formatArgument$cont@111-1(DeployDbArgs args, String additionalParameters, String variables, String argumentName, Unit unitVar) in D:\a\1\s\src\app\Fake.Sql.SqlPackage\Sql.SqlPackage.fs:line 121
   at Fake.Sql.SqlPackage.formatDacPacArguments(DeployDbArgs args, String action, String outputPath, String additionalParameters, String variables) in D:\a\1\s\src\app\Fake.Sql.SqlPackage\Sql.SqlPackage.fs:line 152
   at Fake.Sql.SqlPackage.deployDb(FSharpFunc`2 setParams) in D:\a\1\s\src\app\Fake.Sql.SqlPackage\Sql.SqlPackage.fs:line 175
   at <StartupCode$FSI_0003>.$FSI_0003.main@()

Known workarounds

Compiling the code locally fixes the issue. It's only on the nuget package that this is an issue.

Related information

Operating system: Win11
.NET Runtime, CoreCLR or Mono Version: .NET 5
Version of FAKE (4.X, 5.X): 5.20.4

What is curious is the exception above: The stack trace starts on line 175 of Sql.SqlPackage.fs. Yet according to the latest code (unchanged since April 2021) this is an empty line.

isaacabraham commented 2 years ago

Another dev on our team can also repro this.

isaacabraham commented 2 years ago

Interestingly, I've just tried against alpha004 of 5.21 and the issue no longer occurs. What's changed?

isaacabraham commented 2 years ago

Ok I can now see - there's was an unsafe match against Timeout None that has now been fixed. I'm going to submit a PR to replace the IsSome / None checks with pattern matches to prevent this happening in future.

fsprojects / FAKE