I have a c# library that sends requests to a number of HTTP endpoints and then tries to de-serialize the result into the specified type. An indicative method is defined as:
public virtual async Task<(T, HttpStatusCode)> GetAsync<T>(string resource)
The issue I now have is that I would like to return more contextual information if there is an error response from the endpoint, in the form of an ErrorResponse
object. Refactoring the code base to allow for that to occur would be a substantial change if I were to just update the method signature to return ErrorResponse
as an addition to the returned tuple because c# doesn't allow for methods to be differentiated by return type, e.g.
public virtual async Task<(T, HttpStatusCode, ErrorResponse)> GetAsync<T>(string resource)
Similarly if I introduced OneOf I would still be modifying the return type of the method, not the parameters:
public virtual async Task<(OneOf<T, ErrorResponse>, HttpStatusCode)> GetAsync<T>(string resource)
Clearly returning some kind of response object containing the HttpStatusCode, T
, and any error information would have been more flexible. 20-20 hindsight is a wonderful thing but unfortunately doesn't provide a simple way forward.
public virtual async Task<Response<T>> GetAsync<T>(string resource)
It's possible I could define parallel methods that don't modify the existing methods but provide the additional information.
public virtual async Task<Response<T>> Getv2Async<T>(string resource)
What is a useful/recommended approach to addressing this situation?
2 Answers 2
What is a useful/recommended approach to addressing this situation?
That depends on how set in stone the statement, "20-20 hindsight is a wonderful thing but unfortunately doesn't provide a simple way forward" is. If you are using semantic versioning and you have a good set of automated tests covering your back when making changes, then it may well be a way forward. change the methods to:
public virtual async Task<(T, HttpStatusCode, ErrorResponse)> GetAsync<T>(string resource)
up the major version number, recompile the clients to handle the new signature and re-run those tests...
But if that really isn't an option, then you really only have one choice, if you do not want to introduce a breaking change. And you have covered it already: create a new parallel method.
public virtual async Task<Response<T>> GetWithErrorResponseAsync<T>(string resource)
(I'd strongly recommend a method name that describes what's different, rather than v2
)
And behind the scenes, the v1
implementations can be changed to call the v2
version, decompose that Response
to a tuple and return the latter.
Of course, since these methods are marked virtual
, I assume you have multiple versions in sub classes, in which case this approach could create a lot of work. In which case, it's again worth asking yourself whether the first option really is ruled out or not.
Alternatively, you could introduce a breaking change in another way and just throw exceptions containing the error details. It's inelegant and will still break existing code, but it may be the quickest to retrofit to your solution.
-
Could not just implement a new "T" element which wraps as many outputs as needed? Call it wrapper, call it TO. This doesn't necessarily force OP to change the method signature. Whatever he/she is doing to map responses to "T" types, can be applied to map responses to the new wrapper/TO/type.Laiv– Laiv2019年05月28日 09:54:15 +00:00Commented May 28, 2019 at 9:54
-
1@Laiv, to do that, you'd have to constrain
T
to be a certain "shape" that allows both the client andGetAsync
to understand how to eg store and retrieve anErrorResponse
value fromT
. In the case of C#, which the OP tags this question with, they are currently limited to base classes and interfaces for those type constraints as the language doesn't support structured typing (yet). Such a constraint would still be a breaking change.David Arno– David Arno2019年05月28日 10:17:24 +00:00Commented May 28, 2019 at 10:17 -
Thanks for the thoughtful response - I had discarded the first option without giving it a lot of consideration. On reflection it is probably not too onerous an exercise and avoids a lot of duplication.David Clarke– David Clarke2019年05月28日 21:38:02 +00:00Commented May 28, 2019 at 21:38
You can refactor your method to return a Response object and use a custom deconstructor. Like this:
public class Response<T>{
public T Value{ get; set;}
public HttpStatusCode Status{ get; set;}
public ErrorResponse Error{ get; set;}
public void Deconstruct(out T value, out HttpStatusCode status){
value = Value;
status = Status;
}
public void Deconstruct(out T value, out HttpStatusCode status, out ErrorResponse error)
{
value = Value;
status = Status;
error = Error;
}
}
Any existing code will use the first Deconstruct method.
-
Would that really work? It's a bit "hacky", but very clever if it does.David Arno– David Arno2019年05月29日 12:48:54 +00:00Commented May 29, 2019 at 12:48
-
Yep, it works. I wasn't sure either, so I tested it out in Linqpad and it works just fine.Blake– Blake2019年05月30日 14:55:07 +00:00Commented May 30, 2019 at 14:55
HttpStatusCode
and theErrorResponse
information into an exception? I guess then I would need to refactor the existing code to handle the new exception whereas currently there is some decision logic around the HttpStatusCode. At least if I introduced a new version of the API then I know exactly where the new version is called because it would only be a handful of requests initially.OneOf
.