codegen: Disambiguate when shared response model shared between multiple codes in same group, and handle 'default' error codes#5370
Conversation
|
Taking a quick look, that's not too far from how our legacy code generator handles the same problem, for much the same reasons. Not identical -- our version uses |
|
I'm going to try some other variants next week because I hate the per-case handling, especially on clients. EDIT: Tried something else and I think it ends up a lot nicer |
1c40c04 to
5c29f4a
Compare
5c29f4a to
935f3fc
Compare
e2ecd05 to
bc81b93
Compare
|
Ok I don't hate this now, having tried to use it. Seems ergonomic enough to me. Although I think this is an important one to get right so I'm not gonna push it if there are objections or suggestions for improvement (breaking changes on this in the future could be a massive pita for anyone using it...) I guess the primary candidate for an alternative would be for the 'leaf' types to be
|
|
@adamw Long time coming but I think this might be ready now |
|
Great, thanks :) |
Closing #5284 and #5286
Re-uses the same 'wrapper' declarations we use for response headers, to handle the case where two status codes for the same method declare the same response body. This approach does mean that there's a bunch of
sealed traitss that get created for such endpoints, but they all inherit from the same parentStatusCodeDisambig. A given code is represented by a single object which inherits from the trait for each endpoint that can return that code. This permits some generic handling of errors.Example generated code:
for an endpoint declared as
This handles 'default' codes by mapping to a case class wrapper rather than an object. So, for
... responses: "204": description: "No response" "404": description: Not found content: application/json: schema: $ref: '#/components/schemas/NotFoundError' default: description: Generic error content: application/json: schema: $ref: '#/components/schemas/SimpleError'you get
(if no other 'competing' error codes, a default is more simply reducable, and ends up as just e.g.
.errorOut(jsonBody[SimpleError].description("Generic error").and(statusCode)), but is still included as a param in the error param type)which I think is pretty good? I really wanted to explicitly avoid passing around a raw status code since it wasn't type-safe in terms of the status codes that can be represented on an endpoint... The shared parent trait and common 'tuple companions' make generic handling relatively nice, from what I've tried.
Added a behaviour flag with default false so as to not break existing code.
One issue with this is that if you had, say, 400, 401 and 403, and the 400 had a different response type, you'd still have that extra status code param for the 400 response, even though we could disambiguate without it. Also you can cause a match error at runtime by specifying a status code valid for a response of a different type. But it's a real pain to codegen something that'll work better here, because you still need that unified parent type -- so you'd need to introduce proper wrappers, rather than just tupling it... Since by far the most common use-case for this functionality is gonna be error responses, all the real specs I've seen that don't already have different json schemas for different error status codes use the same model for all error codes, and since generic handling becomes a bit more of a faff with wrappers rather than pairs, I think this still works out pretty well, so I'm happy enough with it.