GuardClauses icon indicating copy to clipboard operation
GuardClauses copied to clipboard

Package Release 4.6 update causes Runtime breaking change: Missing Method Exception

Open DDH-DGillett opened this issue 1 year ago • 10 comments
trafficstars

Environment

  • Windows 22H2
  • .NET SDK Version: 8.07

Subject of the issue

Where there is a hierarchy of code that use Ardalis.GuardClauses updating the top level package, without updating the tranisitive library package to use the same version complies without error, but throws System.MissingMethodException : Method not found: 'System.String Ardalis.GuardClauses.GuardClauseExtensions.NullOrEmpty(Ardalis.GuardClauses.IGuardClause, System.String, System.String, System.String)'.

Steps to Reproduce:

  1. Update top-level package in client solution from v4.5 to v4.6
  2. Version 4.5 remaining in library packages
  3. Run application or tests
  4. Exception is thrown when guard clause is called.

Expected behaviour

Update to top-level package should not cause runtime exception where no changes have been made to top-level code.

Actual behaviour

Calls to guard clauses throw:

System.MissingMethodException : Method not found: 'System.String Ardalis.GuardClauses.GuardClauseExtensions.NullOrEmpty(Ardalis.GuardClauses.IGuardClause, System.String, System.String, System.String)'.

Tested resolution

Updating all libraries using the package to version 4.6 resolved the issue, as did reverting top-level package to 4.5 or removing top-level package and using transitive.

DDH-DGillett avatar Jul 25 '24 23:07 DDH-DGillett

I've seen this as well but I'm not sure how to resolve it. I agree it's a problem and especially nasty since it's only detected at runtime.

This seems related but not sure it's actually a fix: https://stackoverflow.com/a/68486007/13729

The "rules" seem to be here: https://learn.microsoft.com/en-us/nuget/concepts/dependency-resolution#dependency-resolution-rules

image seems to indicate there would at least be a warning, so maybe treating warnings as errors would prevent this?

ardalis avatar Jul 26 '24 19:07 ardalis

My thoughts were a multi-step approach to handle the change:

  1. Update the release notes of 4.6 to note this is breaking change at runtime if there are any transitive dependencies for this package.
  2. Then consideration given to versioning, perhaps this should be 5.0 (since it is breaking) or 8.0 to align with .NET version?
  3. Finally, is it worth considering an alternate approach to resolve this issue entirely? For example via moving away from optional parameters, via refactoring, to method overloads or add new methods that accept the exceptions.

For example:

    public static ReadOnlySpan<char> Empty(this IGuardClause guardClause,
        ReadOnlySpan<char> input,
        string parameterName,
        string? message = null,
        Func<Exception>? exceptionCreator = null)
    {
        if (input.Length == 0 || input == string.Empty)
        {
            Exception? exception = exceptionCreator?.Invoke();

            throw exception ?? new ArgumentException(message ?? $"Required input {parameterName} was empty.", parameterName);
        }
        return input;
    }

Could be split into:

    public static ReadOnlySpan<char> Empty(this IGuardClause guardClause,
        ReadOnlySpan<char> input,
        string parameterName,
        string? message = null)
    {
        if (input.Length == 0 || input == string.Empty)
        {
           throw new ArgumentException(message ?? $"Required input {parameterName} was empty.", parameterName);
        }
        return input;
    }

And:

/// <summary>
    /// Throws an <see cref="ArgumentException" /> or a custom <see cref="Exception" /> if <paramref name="input" /> is an empty string.
    /// </summary>
    /// <param name="guardClause"></param>
    /// <param name="input"></param>
    /// <param name="exceptionCreator">Optional. Custom exception</param>
    /// <returns><paramref name="input" /> if the value is not an empty string.</returns>
    /// <exception cref="ArgumentException"></exception>
    /// <exception cref="Exception"></exception>

    public static ReadOnlySpan<char> Empty(this IGuardClause guardClause,
        ReadOnlySpan<char> input,
        Func<Exception> exceptionCreator)
    {
        if (input.Length == 0 || input == string.Empty)
        {
            throw exceptionCreator.Invoke();
        }
        return input;
    }

DDH-DGillett avatar Jul 27 '24 01:07 DDH-DGillett

I think the right next step is to publish a v5 version and if possible update the release notes for v4.6 as you suggest.

I've added some more notes here as well: https://github.com/ardalis/GuardClauses/pull/350 which was the PR that added the breaking change (at my request).

As for splitting the methods, I see your point in that any particular call is either going to use a custom exception message or a custom exception (factory). I'll consider that although it does double the size of the codebase...

ardalis avatar Jul 30 '24 13:07 ardalis

Hm, I think this needs to be reverted in a patch version, then added back in a new major version with a clear method contract change. We are experiencing ripple effects due to this in our platform (we use "4.*" for our versioning, which now we need to rethink).

leus avatar Sep 03 '24 13:09 leus

Ok, I've updated the release notes on 4.6 (later than I'd meant to - sorry!) and published a new v5.0 version which only has one change in it (non-functional - change to Obsolete message for old AgainstExpression guards) relative to the 4.6 release which had the breaking change(s) in it.

Please let me know if this resolves any issues you have or if there are still problems remaining.

My recommendation if you don't want to change anything and you've been on 4.x for a while is:

Stay on 4.5

If you are willing to make some (minor) changes to your code to accommodate the breaking changes introduced in 4.6:

Update to the latest 5.x version.

ardalis avatar Sep 30 '24 18:09 ardalis

@ardalis

  1. Has the "Missing Method Exception" been fixed in any version, if so, which version(s)?
  2. Is there a version or versions where we can pass a custom exception and also not receive the "Missing Method Exception" message?
  3. What are the minor changes to our code that accommodates the breaking changes?

Thanks!!

Matt

mwasson74 avatar Oct 01 '24 12:10 mwasson74

  1. This issue remains; I'm still able to repro it using a parent project targeting 5.0 and a dependent one targeting 4.5.
  2. Using 4.5 for both or using 5.0 for both (parent and dependents) works fine. A mismatch spanning versions 4.x and 5 (or 4.6) triggers the behavior.
  3. My comment about minor changes assumed you didn't have the mismatch in versions described above. In which case it would involve passing a Func<Exception> if you wanted to specify a custom exception, for instance.

ardalis avatar Oct 01 '24 14:10 ardalis

I'm not sure I'm going to be able to resolve the missing method exception across the v4.5 - vLater divide at this point. Adding method overloads just leads to ambiguous calls because of the use of nullable/default parameters on the methods.

I probably could make it work by explicitly creating overloads for every supported parameter combination, maybe, instead of using default parameters. But it would result in a lot more code and might be less flexible.

How important is it to folks to try to achieve backward compatibility with 4.5 and earlier in versions 4.6 and later (at this point it would come in a new version 5.1 or 6.0 or something)? Leave a 👍 or something if you need this, and I'll consider it. Otherwise, stick to 4.5 if anything is on 4.5 or earlier and latest for everything otherwise.

ardalis avatar Oct 01 '24 14:10 ardalis

I don't think this is fully resolved in v 5 either. I am seeing the "missing method excpetion" in the following scenario:

.Net8 App (XUnit Test)
├── Ardalis v5.0
├── .Net8 project ref
│   │   ├── Ardalis v5.0
│   │   ├── .NetStnd 2.0 (lib/nuget)
│   │   │     ├── Ardalis v5.0

When I call a member defined in the .NetStnd lib from XUnit test that uses Guard clauses internally to ctor, it throws. This type passes unit tests in the lib's repo, and in a different .NetFW 4.8.1 unit test code.

jasells avatar May 02 '25 01:05 jasells

Maybe this targeting config has something to do with it? It seems like it could explain the different results I've seen between FW 4.8.1 unit tests vs. .Net8 unit tests

https://github.com/ardalis/GuardClauses/blob/35cd1cfcf53fbbb32d88aabe0dc4c0f620d5a0f5/src/GuardClauses/GuardClauses.csproj#L6

I'm not sure why you would target both Stnd 2.0 and 2.1? Not to mention .net 8 on top of both of those, since Stnd 2.0 is compatible with both of the others... Makes me suspect that one of the targets in the package is not actually working but the in-repo unit tests (.Net8) are missing the failing scenario? Could be missing the issue by only testing the .Net8 version of the lib when consumed by .Net8 app-code?

A repro that might shed light is:

Create another unit test project (FW 4.8 or 4.8.1) in this repo, move current tests to a shared-code project so that both existing .Net8 and new FW test project can compile and run the same tests in the respective runtimes. Add a .Netstandard2.0 lib project that uses the guard clauses lib, and write tests for this new lib as well.

jasells avatar May 05 '25 13:05 jasells

I noticed another tricky behavior:

  • A test assembly was using version 5.0.0 and was calling a class implemented in another assembly using the pkg version 4.5.0.
  • both assemblies are .net 8
  • The class under test had a generic method that called another private generic method: the tests were green.
  • After a refactoring, the private method was moved to a base class: from that point on, the tests failed with a System.MissingMethodException.
  • By aligning the pkg versions in both assemblies, the tests were green again.

This behavior was even more difficult to identify because, from the stack trace, the error seemed somehow related to the fact that the private generic method was defined in a type (base class) different from the one initially triggered by the test (the class that extends it), while the error was generated by the call to the Guard.Against.Null(myParamInstance) method used within the private method moved to the base class.

giovanni-lisitano-lc avatar Jul 28 '25 17:07 giovanni-lisitano-lc