Vonage/vonage-dotnet-sdk

NuGet package has missing/incorrect dependencies

OronDF343 opened this issue · 4 comments

OronDF343 commented

Describe the bug
The NuGet package Vonage 6.1.0 is missing a depndency on Enums.NET >= 4.0.0 (and possibly other packages). This causes an exception when sending requests with VonageClient (for example, WhatsApp templates).
The cause of the issue is that the dependencies in Vonage.csproj do not match those in Vonage.Common.csproj.

To Reproduce
Steps to reproduce the behavior:

  1. Create a new .NET 6.0 console application
  2. Install package Vonage 6.1.0
  3. Write some code that uses VonageClient in Program.cs:
using Vonage;
using Vonage.Messages.WhatsApp;

var creds = Vonage.Request.Credentials.FromApiKeyAndSecret(...);
var vonage = new VonageClient(creds);
var item = new WhatsAppTemplateRequest
{
    ...
};
var res = await vonage.MessagesClient.SendAsync(item);
Console.WriteLine(res.MessageUuid);
  1. Debug the application. An exception is thrown: System.IO.FileNotFoundException: Could not load file or assembly 'Enums.NET, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7ea1c1650d506225'. The system cannot find the file specified.

Expected behavior
The NuGet package should function correctly out of the box, without manually installing additional 3rd-party packages.

Stack trace

System.IO.FileNotFoundException
  HResult=0x80070002
  Message=Could not load file or assembly 'Enums.NET, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7ea1c1650d506225'. The system cannot find the file specified.
  Source=Vonage.Common
  StackTrace:
   at Vonage.Common.Serialization.EnumDescriptionJsonConverter`1.Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options)
   at System.Text.Json.Serialization.JsonConverter`1.TryWrite(Utf8JsonWriter writer, T& value, JsonSerializerOptions options, WriteStack& state) in /_/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs:line 355
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.GetMemberAndWriteJson(Object obj, WriteStack& state, Utf8JsonWriter writer) in /_/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs:line 309
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryWrite(Utf8JsonWriter writer, T value, JsonSerializerOptions options, WriteStack& state) in /_/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs:line 306
   at System.Text.Json.Serialization.JsonConverter`1.TryWrite(Utf8JsonWriter writer, T& value, JsonSerializerOptions options, WriteStack& state) in /_/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs:line 440
   at System.Text.Json.Serialization.JsonConverter`1.WriteCore(Utf8JsonWriter writer, T& value, JsonSerializerOptions options, WriteStack& state) in /_/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.WriteCore.cs:line 47
   at System.Text.Json.Serialization.JsonConverter`1.WriteCoreAsObject(Utf8JsonWriter writer, Object value, JsonSerializerOptions options, WriteStack& state) in /_/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.WriteCore.cs:line 36
   at System.Text.Json.JsonSerializer.WriteCore[TValue](Utf8JsonWriter writer, TValue& value, JsonTypeInfo`1 jsonTypeInfo) in /_/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Helpers.cs:line 46
   at System.Text.Json.JsonSerializer.WriteString[TValue](TValue& value, JsonTypeInfo`1 jsonTypeInfo) in /_/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.String.cs:line 141
   at Vonage.Messages.MessagesClient.<>c.<SendAsync>b__4_1(Object value)
   at Vonage.Request.ApiRequest.<DoRequestWithJsonContentAsync>d__18`1.MoveNext()
   at Program.<<Main>$>d__0.MoveNext() in ...Program.cs:line 21
   at Program.<Main>(String[] args)

Desktop:

  • Windows 10 22H2
  • Visual Studio 2022 Professional 17.5.3
  • .NET SDK 6.0.407, Runtime 6.0.15

Additional context
In my opinion, the problem is more than just 1 missing dependency. See Vonage.csproj and Vonage.Common.csproj - The dependencies of the latter do not appear in the NuGet package.

Now, take a look at how many transient dependencies are installed with this package:

Installing:

jose-jwt.4.1.0
Microsoft.AspNetCore.WebUtilities.2.2.0
Microsoft.Extensions.Configuration.7.0.0
Microsoft.Extensions.Configuration.Abstractions.7.0.0
Microsoft.Extensions.Configuration.FileExtensions.7.0.0
Microsoft.Extensions.Configuration.Json.7.0.0
Microsoft.Extensions.DependencyInjection.7.0.0
Microsoft.Extensions.DependencyInjection.Abstractions.7.0.0
Microsoft.Extensions.FileProviders.Abstractions.7.0.0
Microsoft.Extensions.FileProviders.Physical.7.0.0
Microsoft.Extensions.FileSystemGlobbing.7.0.0
Microsoft.Extensions.Logging.7.0.0
Microsoft.Extensions.Logging.Abstractions.7.0.0
Microsoft.Extensions.Options.7.0.0
Microsoft.Extensions.Primitives.7.0.0
Microsoft.Net.Http.Headers.2.2.0
Microsoft.NETCore.Platforms.1.1.1
Microsoft.NETCore.Targets.1.1.0
Microsoft.Win32.Primitives.4.3.0
Microsoft.Win32.Registry.4.3.0
Newtonsoft.Json.13.0.2
runtime.debian.8-x64.runtime.native.System.Security.Cryptography.OpenSsl.4.3.2
runtime.fedora.23-x64.runtime.native.System.Security.Cryptography.OpenSsl.4.3.2
runtime.fedora.24-x64.runtime.native.System.Security.Cryptography.OpenSsl.4.3.2
runtime.native.System.4.3.0
runtime.native.System.Net.Http.4.3.0
runtime.native.System.Security.Cryptography.Apple.4.3.1
runtime.native.System.Security.Cryptography.OpenSsl.4.3.2
runtime.opensuse.13.2-x64.runtime.native.System.Security.Cryptography.OpenSsl.4.3.2
runtime.opensuse.42.1-x64.runtime.native.System.Security.Cryptography.OpenSsl.4.3.2
runtime.osx.10.10-x64.runtime.native.System.Security.Cryptography.Apple.4.3.1
runtime.osx.10.10-x64.runtime.native.System.Security.Cryptography.OpenSsl.4.3.2
runtime.rhel.7-x64.runtime.native.System.Security.Cryptography.OpenSsl.4.3.2
runtime.ubuntu.14.04-x64.runtime.native.System.Security.Cryptography.OpenSsl.4.3.2
runtime.ubuntu.16.04-x64.runtime.native.System.Security.Cryptography.OpenSsl.4.3.2
runtime.ubuntu.16.10-x64.runtime.native.System.Security.Cryptography.OpenSsl.4.3.2
System.Buffers.4.5.0
System.Collections.4.3.0
System.Collections.Concurrent.4.3.0
System.Collections.Immutable.7.0.0
System.ComponentModel.4.3.0
System.ComponentModel.Annotations.5.0.0
System.ComponentModel.Composition.7.0.0
System.Diagnostics.Debug.4.3.0
System.Diagnostics.DiagnosticSource.4.3.0
System.Diagnostics.Process.4.3.0
System.Diagnostics.Tracing.4.3.0
System.Formats.Asn1.5.0.0
System.Globalization.4.3.0
System.Globalization.Calendars.4.3.0
System.Globalization.Extensions.4.3.0
System.IO.4.3.0
System.IO.Abstractions.19.1.18
System.IO.FileSystem.4.3.0
System.IO.FileSystem.Primitives.4.3.0
System.Linq.4.3.0
System.Net.Http.4.3.4
System.Net.Primitives.4.3.0
System.Reflection.4.3.0
System.Reflection.Primitives.4.3.0
System.Resources.ResourceManager.4.3.0
System.Runtime.4.3.0
System.Runtime.CompilerServices.Unsafe.6.0.0
System.Runtime.Extensions.4.3.0
System.Runtime.Handles.4.3.0
System.Runtime.InteropServices.4.3.0
System.Runtime.Numerics.4.3.0
System.Security.Cryptography.Algorithms.4.3.1
System.Security.Cryptography.Cng.5.0.0
System.Security.Cryptography.Csp.4.3.0
System.Security.Cryptography.Encoding.4.3.0
System.Security.Cryptography.OpenSsl.5.0.0
System.Security.Cryptography.Primitives.4.3.0
System.Security.Cryptography.X509Certificates.4.3.0
System.Text.Encoding.4.3.0
System.Text.Encoding.Extensions.4.3.0
System.Text.Encodings.Web.7.0.0
System.Text.Json.7.0.0
System.Threading.4.3.0
System.Threading.Tasks.4.3.0
System.Threading.Thread.4.3.0
System.Threading.ThreadPool.4.3.0
TestableIO.System.IO.Abstractions.19.1.18
TestableIO.System.IO.Abstractions.Wrappers.19.1.18
Vonage.6.1.0
Yoh.Text.Json.NamingPolicies.1.0.0

Here's what's wrong in my opinion:

  • All of the System.* packages with version 4.3.x are not needed for any currently supported .NET platform. They are used only for .NET Core 1.x. (Vonage references some of these, which bring in all of the others via legacy DNX5.0 TFMs. .NET Framework is not affected by this)
  • Vonage references both Newtonsoft.Json (directly) and System.Text.Json (via Yoh.Text.Json.NamingPolicies). There is no reason to reference both of these at the same time outside of migration of legacy code (same functionality).
  • The legacy package Microsoft.AspNetCore.WebUtilities is referenced directly (no longer maintained since .NET Core 2.x)

Here's what I think needs to happen:

  1. The dependencies of the NuGet package should be the union of the dependencies of Vonage.csproj and Vonage.Common.csproj.
  2. Please consider using multi-targeting with conditional references. This is the approach recommended by Microsoft, and will help with utilizing modern .NET features better.
  3. Unless .NET Core 1.x compatibility is required, consider removing all of the System.* dependencies that are version 4.3.x. Especially System.Net.Http as referencing this package is known to cause issues with binding redirects in .NET Framework.
  4. Verify whether it is actually necessary to reference both Newtonsoft.Json and System.Text.Json
Tr00d commented

Hi @OronDF343,
Thanks for letting us know. I'll take a look into that today, and I'll get back to you asap.

Tr00d commented

@OronDF343 I released a new version, v6.2.0, today that fixes the problem. Thanks a lot for highlighting it!
Thanks also for bringing up all these points. Here's what I can share at the moment:

  • System packages with version 4.3.x => We had plenty of those. The main project was migrated from specific frameworks to netstandard2.0 a few versions ago, and dependency cleaning has been forgotten.
  • Newtonsoft vs. System.Text.Json => Good catch - the goal is to migrate to System.Text.Json, but we're iterating on that, given it's a high investment to do everything at once. This is a temporary situation (hard to give a forecast, though), and it explains why we currently have both dependencies. Still, it shouldn't affect you from a consumer perspective, given it's all in the internals.
  • Microsoft.AspNetCore.WebUtilities => The only reason we depend on that is to benefit from QueryHelpers. Sticking to netstandard2.0 means we can't benefit from Net6.0 and above, and that's a shame. I'll keep it on my radar to maybe re-implement something internally and remove this dependency.
  • The package dependency should be the union of dependencies => I absolutely agree. I thought it was currently the case, but it looks like this one slipped through the net. The way the package is created is a bit weird regarding nested projects, and it requires us to do the following to include Vonage.Common

    vonage-dotnet-sdk/Vonage/Vonage.csproj

    Lines 89 to 92 in 980bff4

    <_PackageFiles Include="$(OutputPath)\Vonage.Common.dll">
    <BuildAction>None</BuildAction>
    <PackagePath>lib\netstandard2.0</PackagePath>
    </_PackageFiles>
  • Multi-targeting =>Indeed, but this shouldn't be required for us anymore. As I mentioned above, the package only complies towards netstandard2.0, and this should make it easy for us.

I hope I have brought some light on the situation.
Happy to talk about it further 😄 And thanks again for reporting the issue!

OronDF343 commented

@Tr00d Thanks a lot for the quick response! The new version improves a lot.

Looking into things further, I agree that multi-targeting would not do much for this package as it is today. If, however, it becomes desirable to use it in the future (e.g. to take advantage of net6.0 features conditionally), this workaround will work for including the Vonage.Common dll in the package per target framework automatically.

Tr00d commented

@OronDF343 Thanks for that! I'll keep an eye on it.
Normally, this is a temporary situation. We have another package generated from this repository (Vonage.Server), representing a beta for our Video features. The package Vonage.Common appeared to allow code reuse and avoid duplication. Once the beta "ends", we shall move back to a single project.