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

[Discussion] Announcement: TryParse and BindAsync discovery on Map* parameters will throw for invalid methods #36810

Closed
2 tasks done
BrennanConroy opened this issue Sep 21, 2021 · 2 comments

Comments

@BrennanConroy
Copy link
Member

BrennanConroy commented Sep 21, 2021

Discussion for announcement: aspnet/Announcements#472

Description

Starting in RC2, when we look for TryParse or BindAsync methods on your parameter types if we don't find a valid one we will also look for invalid ones and throw at startup to let you know that you might have written your method signature incorrectly to avoid unexpected behavior.

Version

.NET 6 RC2

Previous behavior

// Todo.TryParse is not in a valid format, will try to bind from body as json instead
app.MapPost("/endpoint", (Todo todo) => todo.Item);

public class Todo
{
    public string Item { get; set; }

    public static bool TryParse(string value) => true;
}

New behavior

We have now changed it so that if we see a public TryParse or BindAsync that doesn't match the expected syntax we will throw on startup. The above example would throw an error similar to:

TryParse method found on Todo with incorrect format. Must be a static method with format
bool TryParse(string, IFormatProvider, out Todo)
bool TryParse(string, out Todo)
but found
Boolean TryParse(System.String)

Type of breaking change

  • Binary incompatible: Existing binaries may encounter a breaking change in behavior, such as failure to load/execute or different run-time behavior.
  • Source incompatible: Source code may encounter a breaking change in behavior when targeting the new runtime/component/SDK, such as compile errors or different run-time behavior.

Reason for change

This change was made so that developers would be aware when they wrote a BindAsync or TryParse method that it wasn't in a valid format. Otherwise the framework would fallback to assuming the parameter is json from the body and could result in unexpected behavior.

Recommended action

It is possible your types have a BindAsync or TryParse with different syntax for other reasons besides parameter binding and will now throw at startup. There are multiple options to avoid this behavior:

  • Make your BindAsync or TryParse method internal or private
  • Add a new BindAsync or TryParse method that is in the syntax we are looking for (we ignore invalid methods if we find a valid one)
  • Mark your parameter as [FromBody]

Affected APIs

All IEndpointRouteBuilder.Map*(...) methods are affected by this change, e.g. app.MapGet(...) and app.MapPost(...).
And RequestDelegateFactory.Create(...)

@fowl2
Copy link

fowl2 commented Sep 23, 2021

This is technically both source and binary breaking, not just binary, right?

@ghost
Copy link

ghost commented Nov 22, 2021

Thank you for contacting us. Due to a lack of activity on this discussion issue we're closing it in an effort to keep our backlog clean. If you believe there is a concern related to the ASP.NET Core framework, which hasn't been addressed yet, please file a new issue.

This issue will be locked after 30 more days of inactivity. If you still wish to discuss this subject after then, please create a new issue!

@ghost ghost closed this as completed Nov 22, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Dec 22, 2021
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants