WithMany: add exception information

[yecril71pl]

Please explain that the method will throw an InvalidOperationException when one of the navigations has not been not specified.

---

Document Details

⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

• ID: 0a08def8-61eb-e1b7-1f1f-71ff63daf959
• Version Independent ID: 4e6af8c7-27a3-95d6-ef06-e8f2cdb06af0
• Content: CollectionNavigationBuilder<TEntity,TRelatedEntity>.WithMany Method (Microsoft.EntityFrameworkCore.Metadata.Builders)
• Content Source: dotnet/xml/Microsoft.EntityFrameworkCore.Metadata.Builders/CollectionNavigationBuilder`2.xml
• Product: ef-core-api
• GitHub Login: @dotnet-bot
• Microsoft Alias: avickers

[ajcvickers]

Duplicate of https://github.com/dotnet/efcore/issues/20086#issuecomment-592704875

[yecril71pl]

It is not a duplicate.

• That one: describe exceptions that may be caught.
• This one: describe what not to do or you will get a run time exception.

These use cases are fundamentally different. Note that it would be much better to catch such errors during build.

[ajcvickers]

@yecril71pl As a team, we discussed documenting exceptions like this, and concluded we could better use our time doing other things. This exception is an "exception that should not be caught as part of normal flow control," which is exactly the case discussed in issue #20086.

[yecril71pl]

@yecril71pl As a team, we discussed documenting exceptions like this, and concluded we could better use our time doing other things. This exception is an "exception that should not be caught as part of normal flow control," which is exactly the case discussed in issue #20086.

The discussion you refer to does not address, or even acknowledge, the use case that I have brought to your attention.

[ajcvickers]

@yecril71pl Regardless what it says in the text or how you interpret it, the discussion itself did cover this case. At best, we could update that issue to indicate that it explicitly covers this case as well, but I don't believe that has much value either.

[yecril71pl]

What has much value is to tell client developers what not to do. If you have no time for that, I could do that. However, the problem with my doing that is that the result will necessarily be incomplete because I can describe problems only when I bump into them at run time—and I sincerely hope I shall not bump into all of them.

[roji]

@yecril71pl consider that if we start documenting this kind of exception, we'd have to start documenting every time a null value is passed where it is not expected (since that triggers an exception). As @ajcvickers wrote, we don't believe there's value in this kind of exception; quite to the contrary, IMHO it pollutes API docs and makes them more difficult to read. Programmers are expected to get the exception as they're coding (or running tests), and consequently fix their code accordingly; docs wouldn't help much with that flow - we prefer ensuring that exception messages are clear and contain instructions.

It's also not a good idea to only partially document the API surface, leaving some areas undocumented; aside from being inconsistent it would possibly give an impression that this category of exceptions are thoroughly documented, when in fact they are not.

[yecril71pl]

@yecril71pl consider that if we start documenting this kind of exception, we'd have to start documenting every time a null value is passed where it is not expected (since that triggers an exception). As

Null values hopefully already are handled by checking solid references at compile time. As I said, it would be best if we could check everything that way; the problem with tests is they may fail to trigger exceptions, depending on the data and the environment.

Not allowing arrays for navigation, for example, is a design decision that is hard-coded and not reflected in the API. It should be documented somewhere.

Regarding the present issue—the situation is somewhat better; the handout explains EF must find out a way to connect entities both ways and how it must be helped in some cases—although the mechanics of this process (esp. what must be said and what can be recognised) is not fully specified, leaving the programmer to a trial-and-error process. Fortunately, in most cases whether the model is workable or not will be decided at start-up, so there is little chance of missing an error here.

[roji]

Not allowing arrays for navigation, for example, is a design decision that is hard-coded and not reflected in the API. It should be documented somewhere.

As @ajcvickers wrote above (and as I tried to explain), we discussed this and decided against documenting "incorrect input" exceptions. A user getting that exception would know to correct their code, and it's quite unlikely the docs would help avoid this (it assumes the user is pre-checking the docs with every line of code they write). Even if they did, there would be little value compared to the user just getting the exception and figuring it out from there.

If you see cases where our exceptions aren't clear about what the error is, then I think that's something we we would improve. But going over every method in the public API surface and painstainkingly documenting every possible way it could be given wrong inputs just doesn't make a lot of sense for us.

[yecril71pl]

Not allowing arrays for navigation, for example, is a design decision that is hard-coded and not reflected in the API. It should be documented somewhere.

As @ajcvickers wrote above (and as I tried to explain), we discussed this and decided against documenting "incorrect input" exceptions. A user getting that exception would know to correct their code, and it's quite unlikely the docs would help avoid this (it assumes the user is pre-checking the docs with every line of code they write). Even if they did, there would be little value compared to the user just getting the exception and figuring it out from there.

That only works if the user getting the exception is the developer, which need not be the case, and this is not the case I am worried about.

[roji]

Allowing arbitrary exceptions to bubble up to end users (which aren't developers) is generally a bad idea, and would imply the developer hasn't tested their code in any way. Testing is obviously essential, and would uncover far more possible bugs than we could possibly document in any docs. Moreover, it seems quite unlikely that developers would preemptively examine our API docs for possible exceptions, when they haven't bothered to write a single test.

In any case, this is a discussion we had internally, and we made the decision this way - I don't think there's any new info here that would make us change our minds.

[yecril71pl]

Allowing arbitrary exceptions to bubble up to end users (which aren't developers) is generally a bad idea, and would imply the developer hasn't tested their code in any way. Testing is obviously essential, and would uncover far more possible bugs than we could possibly document in any docs. Moreover, it seems

Testing is not guaranteed to uncover all bugs.

[roji]

Indeed, but neither is up-front documentation of all possible thrown input exceptions. We don't believe there's enough value in this.

[yecril71pl]

Indeed, but neither is up-front documentation of all possible thrown input exceptions. We don't believe there's enough value in this.

I do not think we should document all possible thrown exceptions. We should, however, document all exceptions that we throw ourselves.

[roji]

I don't see how it matters from a user perspective if an exception is thrown directly from EF code or not... An exception is an exception..

But in any case, as I said, we've made a decision not to do that.

[ajcvickers]

Discussed again in triage. Closing as by-design.

[yecril71pl]

I don't see how it matters from a user perspective if an exception is thrown directly from EF code or not... An exception is an exception..

An exception that is not thrown by EF is either trivial or unworkable (e.g. the database server becomes unavailable). OTOH, an exception thrown by EF means you have done something you were not supposed to do in your code, so I would like to know what these things are.

[roji]

EF could be passing your input into some .NET API (or 3rd-party), and that could be throwing for invalid input. I don't see what difference it makes whether EF is the one throwing the exception or a dependency.

[yecril71pl]

The difference is for the technical writer, 3rd-party libraries are beyond our control and a moving target so it is impossible to document them. If those are insufficiently documented, they will take the flak, not us. Of course, it may happen that EF-invalid input gets GIGO-transformed and passed to 3rd-party libraries only to be rejected there by throwing a technical exception. Such an exception should be caught and wrapped in our exception. I believe this is what currently happens. But if the input is EF-valid, there is nothing we can do.

[roji]

In the end, API documentation is intended to help developers using that API, and those developers don't care about whether an exception originates in EF Core or some dependency inside it; that's an irrelevant implementation detail as far as our public API consumers are concerned.

[yecril71pl]

I am also a developer and I care (because the origin of the exception is where I look for an explanation), so it must be all the developers of the world except one.

[roji]

But as a developer using EF, how would you know if the origin of, say, an InvalidOperationException is within the EF source code, or some 3rd-party library used internally by EF (or even the runtime)? Why should you care?

[yecril71pl]

An exception includes a stack trace, so I know whether this was thrown by EF or not. I go down the stack trace to find the first public API, then I go to the documentation for that API to learn more about that particular exception. Of course, if it is just a wrapper around some hardware failure, no help is to be expected. But sometimes the exception is by design, which is an interesting case to learn more about.

[roji]

So you're describing a scenario where the scenario has already gotten an exception, rather than looking up the API docs before that event. As I wrote above, my view is that whatever exceptions bubble out of EF Core's public APIs should provide enough info to unblock the developer, without them needing to check any additional API docs. That's why exceptions contain messages.

In addition, even when an exception is coming from a dependency - and isn't wrapped - it could still indicate a problem with your input without actually shedding any light on it. For example, you may be giving EF some invalid parameter value which causes it to call some dependency method in a place where it shouldn't. You'd then get an exception from the dependency, but that exception wouldn't tell you anything - the problem with the fact that your parameter caused EF Core to call the lower-level API.

So to summarize: exactly which internal component originally threw the exception doesn't seem interesting to me - that's an implementation detail as far as the user is concerned. The important thing is that a the developer be able to understand what they've done wrong, and that should happen reasonably well from the exception itself. If the developer already sees an exception, they shouldn't need to go to any docs to understand it.

[yecril71pl]

So you're describing a scenario where the scenario has already gotten an exception, rather than looking up the API docs before that event.

You asked:

how would you know if the origin of, say, an InvalidOperationException is within the EF source code?

I answered that specific question. I included nothing in my answer to indicate that I do not use API documentation to learn about possible exceptions before I see them (or even use the API). While I certainly do not do that with every API, I do it whenever I am going to use one for the 1st time, or to refresh my memory when I feel uncertain.

[roji]

OK. It does feel like this conversation has run its course - in any case I've pretty much written everything I had to say above.