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

Return detailed errors from B, same as C2 #218

Open
nickevansuk opened this issue Sep 21, 2021 · 1 comment
Open

Return detailed errors from B, same as C2 #218

nickevansuk opened this issue Sep 21, 2021 · 1 comment
Labels
CR3 Issues relating to CR3

Comments

@nickevansuk
Copy link
Contributor

nickevansuk commented Sep 21, 2021

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!

@nickevansuk nickevansuk added the CR3 Issues relating to CR3 label Sep 21, 2021
@nathansalter
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CR3 Issues relating to CR3
Development

No branches or pull requests

2 participants