ietf-wg-httpapi / rfc7807bis

Revision of RFC7807: HTTP Problem Details
Other
20 stars 8 forks source link

Multiple problems #6

Closed mnot closed 2 years ago

mnot commented 3 years ago

Sometimes an API will want to report more than one problem in a response. 7807 explains that this can be done in extensions, but does not natively allow multiple problems to be conveyed, because there was concern that doing so would make it too easy to contradict the semantics of the status code.

Since publication, developers have still wanted a generic solution to this. A few possible directions we could go on:

  1. Do nothing
  2. Improve the documentation regarding how to handle multiple problems in extensions
  3. Introduce a way to natively convey multiple problems, as long as they share the same status code semantics
  4. Something else?
sazzer commented 3 years ago

I'm somewhat curious how often multiple different problems really happen, as opposed to multiple different facets of the same problem.

By "multiple different facets of the same problem", I'm meaning things like request validation failures. A request might be invalid because of multiple different fields in the incoming JSON(/whatever content type is being used). But the overall problem is still "Validation Failed". It's just that it failed for several reasons.

The real multiple problems would, in my mind at least, be something like:

The "multiple facets" case is relatively simple to support. Indeed, I'm sure the wider community have a myriad of different ways to do it already, but obviously a standard way to achieve it would be better.

The "multiple problems" case is much harder, simply because of the different status codes that can come up. But my (potentially naive) suspicion is that it's also less useful.

serialseb commented 3 years ago

I hope it's useful, here is the scenario of what we currently do (it's a work in progress).

We currently retrieve various problem documents from various api calls that are done in a scatter/gather scenario, unbeknownst to the client. We aggregate those potential multiple problems:

The status code in the case of an aggregate is set according to those rules:

It's not ideal but it allows us to communicate to the client why their request failed, with information about each dataset that may have rejected a request.

mnot commented 3 years ago

@serialseb that's an interesting pattern. I could see recommending that in the spec, and advising problem definitions to think carefully about accommodating multiple instances of their details...

gervaisb commented 3 years ago

I agree with the facets ideas. And I would go further by asking to provide a format for validation errors.

gabesullice commented 3 years ago

The "multiple problems" case is much harder, simply because of the different status codes that can come up. But my (potentially naive) suspicion is that it's also less useful.

I agree with this. I find it hard to imagine that clients will actually attempt to resolve all problems in a multi-status response before retrying a request because the implementation seems like it would be overly complicated. @serialseb, in practice, do your clients attempt to resolve multiple the problems or are the problems merely logged?

Option 1 seems most pragmatic to me. I.e. explain that a problem type can use extensions to define an array of subproblems with examples. Then explain that if the problems are not subproblems of a broader category, the server should choose the most critical problem.

If the server doesn't choose the most critical problem, then we will likely have to provide a way to order/prioritize problems in the media type to guide the client. For example, if a server responds with both the insufficient-storage and invalid-field error types, the client will have to understand that it's a waste of time to fix the validation error because the server is out of space. This still permits a client to fix multiple problems iteratively by retrying the request after each problem has been fixed.

serialseb commented 3 years ago

@gabesullice in practice, when there are multiple problems, it comes to a human to analyse. That analysis can be done iwth raw text, or with structured data. People do structured data all the time, and if you don't have an answer they'll build their own. Now, the rules we use is to allow a simple automated system to take care of the cases where the most specific issues happen (say, two gateways are timing out, it's a timeout, we merge towards a specific status code, automated clietns take care of it). In the cases where they're not, the lack of standard way of communicating a common scenario leads to one of those less than ideal scenarios: try not to step on the spec's turf, and create your own schema (implementation cost reduction through network effect nullified, no way to generiically notify operators as schema is now per api), not give data (worst outcome), or give text and hope someone reads it. Operational support scenarios can be automated in the "let's get a human involved" pathway, and as long as we have no answer for this, no tool vendor will use this, making everything more complex, more isolated, and in turn, more expensive.

If one of the variables is badly written and an invalid URI, and another one does not exist, and i've been teaching people to reuse errors they understand, i now have no standard way to explain that one problem is X and another is Y.

Operational is part of APIs and automation is as important to DevRelOps as it is to just plain APIs.

andrecedik commented 3 years ago

Thank you @serialseb for the insights into how you're handling this issue. It's an interesting approach to the limitations of the original RFC and reflects what we're also trying to 'fix' with our I-D. Do you also return the status attribute within the inner_problems array to let the client know the severity of the problem?

if a server responds with both the insufficient-storage and invalid-field error types, the client will have to understand that it's a waste of time to fix the validation error because the server is out of space

@gabesullice it may be a waste of time at the moment, but when you know beforehand that fixing the space problem won't make the other problem(s) go away one could also think about correcting the invalid-field error before sending a new request.

This still permits a client to fix multiple problems iteratively by retrying the request after each problem has been fixed.

Yes, but isn't that a bad user experience? I'd rather know everything I'm doing wrong right now before making a new request, fixing one problem, making another request, fixing another problem, and so on.

gabesullice commented 3 years ago

Yes, but isn't that a bad user experience?

It's certainly a trade off, but fixing one problem at a time is the best way to debug. Whenever I try to make more than one change at a time, I end up in a more chaotic debugging state than before. It's good practice to make isolated changes that you understand rather than throwing things at the wall to see what sticks.

The trade off is that the media type is less flexible and certain things will be more difficult to convey.

However, in my experience with the JSON:API spec (which supports multiple error objects), server implementers tend to dump useless stack traces into response bodies and try to abuse them to make non-atomic updates. Multiple errors also encourage client implementers to try the kitchen sink approach to debugging.

So it's in the spirit of designing something that makes good practices easy and bad practices difficult that I'm pushing back against supporting multiple problem details.


My suggestion is to finalize this document for the single error case. Then mint an error type called multiple which defines an extension member named instances. The value of instances would be an array of problem objects as defined for the single error case. Perhaps it will add additional extension members for those child objects as well.

To benefit from network effects, the multiple type can be published using the IETF process. By separating that into its own document, you can iterate on that design in a separate document without postponing this one.

serialseb commented 3 years ago

Do you also return the status attribute within the inner_problems array to let the client know the severity of the problem?

Yes we do, but this is only used for operators to gain additional insight.

To benefit from network effects, the multiple type can be published using the IETF process. By separating that into its own document, you can iterate on that design in a separate document without postponing this one.

I'd be receptive to such a proposal

andrecedik commented 3 years ago

So it's in the spirit of designing something that makes good practices easy and bad practices difficult that I'm pushing back against supporting multiple problem details.

I'm with you on making bad practices difficult and good practices easy 😁

My suggestion is to finalize this document for the single error case. Then mint an error type called multiple which defines an extension member named instances. The value of instances would be an array of problem objects as defined for the single error case. Perhaps it will add additional extension members for those child objects as well.

Sounds promising. I'd be willing to support this.

sdatspun2 commented 3 years ago

However, in my experience with the JSON:API spec (which supports multiple error objects), server implementers tend to dump useless stack traces into response bodies and try to abuse them to make non-atomic updates.

I am not a proponent of JSON:API spec, but honestly, this has nothing to do with the multiple error objects. It is due to lack of knowledge/discipline/governance on the API developer's side. Someone can dump exception in detail even today. A few days ago, I had to explain to a very senior Java developer that you just cannot stream Java exceptions in error response of an HTTP API.

My suggestion is to finalize this document for the single error case. Then mint an error type called multiple which defines an extension member named instances. The value of instances would be an array of problem objects as defined for the single error case. Perhaps it will add additional extension members for those child objects as well.

I disagree. Multiple problems could be added using extension mechanism today as well. One of the main reasons for -bis is to provide support for multiple problems without any extension (in order to increase adoption).

mnot commented 3 years ago

My .02 - If we define a multiple type, some implementations are likely to use it by default, even when a single problem is transferred. That seems suboptimal, because it loses the mapping between the HTTP status code and the actual problem type.

Also, a multiple problem type would by necessity have to use a generic status code (probably 207), thereby reducing semantics exposed in HTTP, which isn't something we should be recommending.

sdatspun2 commented 3 years ago

Looks like we are back to square one on the issue of support for multiple problems. Imho, having HTTP status code duplicated in the problem details response body was not desirable. Now, that it is in the RFC, its presence creates a real problem in supporting the multiple problems use case which is also a common use case in any HTTP API accepting requests with a slightly complex schema.

mnot commented 3 years ago

It's not a matter of whether or not the status code is duplicated; it's whether the semantics of the problem are appropriately reflected in the status code.

mnot commented 3 years ago

I think we're gravitating towards the (1) direction outlined above - 'Improve the documentation regarding how to handle multiple problems in extensions'

We could do that by:

mnot commented 3 years ago

Discussed in 111: @mnot to write preliminary PR.

mnot commented 3 years ago

Also, since we now have a registry, we could go ahead and define a problem type that handles multiple instances -- for example, 'json input validation error'. That might help tease out some of the issues here.

FusionFabricUser commented 3 years ago

here is the model followed @Finastra https://fusionfabric.github.io/open-api-standard/Responses_and_Errors.html#finastra-error-message-standards

we are using a fields causes that is defined as an array of object. use case is that for a given failure like a transaction several causes can be implied (ie first field not correct size, second not a number etc )

key design decision would be is this a single object so cause , or an array of object

notice that the cause has been also introduce by Zalando https://github.com/zalando/problem that has been integrated with a Spring extension

thexa4 commented 2 years ago

Wouldn't multipart/related [RFC2112] be a good fit for this?

FusionFabricUser commented 2 years ago

Multi part tend to disapears in the API spaces as far as i can see (unfortunate or not)

@thexa4 , array is a better pattern for API , mutli part is usually attached to concept of differents payload encoding, that here i don't see the point for 99 % of use cases . if ever you requires a dedicated pdf file for the causes , it should go to a functional pattern or a custom extension of it following a keep it simple approach

nnmrts commented 2 years ago

This comment, as most of my comments on issues in repositories, got a bit out of hand and way too long. I'm sorry for that and my limited english vocabulary to make this a bit more compact.


Although this issue seems to go into the opposite direction, I agree with https://github.com/ietf-wg-httpapi/rfc7807bis/issues/6#issuecomment-810272529 by @gabesullice but don't think there should even be a "multiple problems" option. Isn't there always a "root problem"? Or at least a "first problem" or "most important problem"? I don't feel like telling a client about only one problem at a time is bad user experience.

It might seem like a bad user experience from the perspective of a human debugging their request by hand, but even then I personally think it's more logical to solve issues iteratively. Yes, one needs to eventually make multiple requests to fix their request, but considering today's average internet speed and the expected small size of problem details responses, I think it's just fine.

But if we think about a computer program handling a problem details response and trying to "fix it", I think it becomes even more clear that only one problem at a time should be in a response. I realize this might be "opinionated", but a client app with multiple atomic handlers each able to "fix" one type of problem seems more sustainable and logical to me than either one huge handler that "fixes" all types of problems or a lot of handlers each fixing every possible combination of problems.

I think this RFC and APIs should be designed considering that and at the moment it already feels optimal as is in this regard. If an API for some reason still has to respond with "multiple problems", it already can, just as described at the end of section 3:

The ability to convey problem-specific extensions allows more than one problem to be conveyed.

@mnot in the first post of this issue states that the current version of the RFC "does not natively allow multiple problems to be conveyed", but I feel like it actually does. Even the example given after that quote from the RFC "conveys", at least to a human reader, that there are multiple problems, since the value of the title field is "Your request parameters (plural) didn't validate." But actually, and I am trying to avoid going full philosophical here, isn't it true that every collection of problems can be seen as one big problem? And every single one problem could theoretically be split up into multiple problems. Let's take a look at the detail field of the first example: "Your current balance is 30, but that costs 50." Sure, the problem in this case is called "out-of-credit" and the obvious solution proposed is topping up the account with more credits. But this RFC didn't tell us this "obvious" solution, we came up with it and that's how it should be. The problem of not having enough credits on your account could theoretically be split up in the problem of you being a bit forgetful or the problem that the service is expensive or their pricing system isn't transparent enough. I realize this isn't the best example for what I'm trying to say, but I still hope you understand it.

It is and only can be the responsibility of whoever implements the API and whoever implements the client, to write code that knows what problems to expect, how the detail field or an extension field is structured, which of them can be fixed and to then provide solutions for them. So I feel it is also their responsibility to write code that "knows" when to expect multiple problems, or a single one, or both. Even just the presence of an extension field called subProblems or invalid-params can indicate that there are multiple problems that each have to be fixed separately.

I think specifically defining some sort of "multiple" type or field is not the optimal strategy here and goes against the spirit of freedom across the rest of this RFC. I realize the purpose of a standard is not to embrace freedom and doing things completely different to anyone else 😆. But defining a specific structure for conveying that there are multiple problems could be dangerous and actually impede adoption, in my opinion. For example, if my API only detected a single validation error, does my server now need to flip the multiple boolean value? Is invalid-params now still an array with just one object in it, or just one object, so actually a different data structure my client needs to handle specifically? If it's always an array why even use the multiple field? My client will iterate over the array anyway and doesn't care how big it is.

I think keeping this aspect of the RFC as is would be a great opportunity to let the community dynamically decide on a best practice and eventually draft a separate document once there is more consensus and more people worked with this specification in the real world.

I definitely don't think the current specification doesn't hinder anyone from handling multiple problems. In the case of form validation for example, where you might only want to send one request for the whole validation, sure, feel free to let the API respond with an array of validation errors in a custom extension field. A client submitting a form will already expect a specific structure of response anyway. As I said, abstractly seen, a multitude of validation errors is still only one specific "big" problem, namely that there are validation errors. The details of those problems can then be described in either the detail field and/or a custom extension field.

dret commented 2 years ago

going back to @mnot's initial comment, i am in favor of option 0, which is to do nothing. if we go down the path of showing speculative extensions, we kind of "define" them without really defining them. that in my mind is the real risk of showing hypothetical extensions in examples. people will use them, regardless how clearly you say "this is not actually an extension and just for illustration".

mnot commented 2 years ago

So, with @sdatspun2's PR merged a while back, we currently:

  1. Describe how to design extensions to accommodate multiple instances of the same type
  2. Describe how to use 207 Multi-Status to convey multiple problems with different types/status codes
  3. Do not describe how one might convey multiple problems with different types but the same status code.

I think (1) is fine and good.

Right now, (2) seems like it's recommending the pattern, but doesn't define an actual type for use in that situation. That's not great. If we're going to show that pattern, we should probably define the type for use in that situation (especially since we have a registry now). However, I have serious doubts about the wisdom of recommending that pattern, and feel like we should either remove that text, or at least heavily contextualise it.

Regarding (3) -- again, if we want to document that, we should define a type. I tend to think we shouldn't, though.

If we retract (2) and don't attempt (3), we might want to add some text along the lines of:

When an API encounters multiple problems that do not share the same type, it is RECOMMENDED that the most relevant or urgent problem be represented in the response. While it is possible to create generic "batch" problem types that convey multiple, disparate types, they do not map well into HTTP semantics.

dret commented 2 years ago

On 2022-02-07 01:56, Mark Nottingham wrote:

Right now, (2) seems like it's recommending the pattern, but doesn't define an actual type for use in that situation. That's not great. If we're going to show that pattern, we should probably define the type for use in that situation (especially since we have a registry now).

i agree. showing something without defining it in the end will be a "bad definition" because people will take it as shown and run with it.

However, I have serious doubts about the wisdom of recommending that pattern, and feel like we should either remove that text, or at least heavily contextualise it.

my vote goes to removing it.

Regarding (3) -- again, if we want to document that, we should define a type. I tend to think we shouldn't, though.

same here.

When an API encounters multiple problems that do not share the same
type, it is RECOMMENDED that the most relevant or urgent problem be
represented in the response. While it is possible to create generic
"batch" problem types that convey multiple, disparate types, they do
not map well into HTTP semantics.

i like it.

sdatspun2 commented 2 years ago

If we retract (2) and don't attempt (3), we might want to add some text along the lines of:

"When an API encounters multiple problems that do not share the same type, it is RECOMMENDED that the most relevant or urgent problem be represented in the response. While it is possible to create generic "batch" problem types that convey multiple, disparate types, they do not map well into HTTP semantics."

+1

mnot commented 2 years ago

OK. It seems like we agree here -- I'll ping the mailing list and make sure everyone is aware / on board.

LasneF commented 2 years ago

my 2 cents , when you do a REST request you expect to have a single status , this is the HTTP status , here a 4XX or 5XX, for sure not a 207 that is part of 2XX section so a success

HTTP requires it fails or it succeed

what problem RFC provides is a more standard way to return feedback about this status ... this is where 'problem' is used RFC problem describes the http status of error with standardized information (title , details, type)

This status can have multiple root cause this is why having as an array is a good pattern . it is not an array of problem (it wont make sense according to the definition of http problems) but an array of cause. you can have a single explanation of a return that can be explained due to various cause

cause could be ... either free form (easiest) 👍 ,

here is a use case about Form filling that is failing . Cause can be the list of fieldName and Value in default but this does not fit all use cases

{ "type": "https://api.zz.com/validation-error", "title": "The request is invalid", "status": 400, "detail": "Customer Creation Failure", "causes": [ { "title": "The name is too Long", "field": "name", "fieldValue": "AsuperExtraLongName" }, { "title": "Date of Birth is in the future", "field": "dateOfBirth", "fieldValue": "20/20/2200" } ] }

another pattern can be to have recursive causes definition ... but this is too munch prescriptive 👎
{ "type": "https://api.zz.com/validation-error", "title": "The request is invalid", "status": 400, "detail": "Cannot create Order", "causes": [ { "type": "https://api.zz.com/validation-error", "title": "Order Management ; one of the item cannot be fulfilled ", "status": 404, "detail": "Item management , item XXX cannot be found ", causes [ type": "https://api.zz.com/validation-error", "title": "Storage", "status": 400, "detail": "Item XXX is out of stock", } }] }