search

openactive / open-booking-api

Repository for the Open Booking API specification
Other
2 stars 3 forks source link

Return detailed errors from B, same as C2 #218

Open nickevansuk opened 2 years ago

nickevansuk commented 2 years ago

Through debugging many implementations, it has become apparent that the current behaviour of obscuring any error at B behind an UnableToProcessOrderItemError is not desirable.

While originally intended to simplify the design, and encourage good practice API usage, the reality of implementations is that all the logic to render the B response is already being implemented for C2.

Although the spec does state "Any validation errors raised at B regarding any property values must also be raised at C2 (and ideally also C1 if relevant), so it should be very unusual to receive an error at B", the errors themselves are currently hidden at B.

This is particularly apparent in .NET implementations, where the errors are literally hidden, and where returning full errors from B as they do from C2 actually mainly requires the removal of the 5 lines of code that "hide" the errors:

https://github.com/openactive/OpenActive.Server.NET/blob/d792ba5f7818f3a3dfa6763aff3ea08504cc9549/OpenActive.Server.NET/StoreBookingEngine/StoreBookingEngine.cs#L400-L405

The code above demonstrates the problem - currently to make the UnableToProcessOrderItemError error meaningful to anyone attempting to debug an issue at B, it is necessary to attempt to bundle aspects of the would-be full JSON response into the error string. This is both problematic for debugging and confusing to the casual implementer.

The benefits of removing UnableToProcessOrderItemError are tangible, as any issues with specific OrderItems at B can then be clearly identified as they are with C2.

Debugging Open Booking API issues that occur only at B in production is currently a much more difficult exercise than it needs to be (and in some cases not possible, depending on whether the error message actually concats the individual errors as with the .NET example above, or just provides a completely opaque error), and the amount of work to fix this is relatively minimal.

Thoughts welcome!

nathansalter commented 2 years ago

In Playfinder we often find that B requests fail due to changes in the state of the booking system between the proposal being accepted and returned in the feed, and then being picked up and placed. Sometimes the state hasn't finished being persisted to the DB, or some issue on our side fails before we even make the B request and we need to retry it.