lejard-h / chopper

Chopper is an http client generator using source_gen and inspired from Retrofit.
https://hadrien-lejard.gitbook.io/chopper
Other
710 stars 122 forks source link

decode and intercept response order? #6

Closed ericta closed 5 years ago

ericta commented 5 years ago

As I see from the base.dart file, the order will be: [ENCODE -> REQUEST -> DECODE INTERCEPT -> RESPONSE INTERCEPT] But from my point of view, it should be [ENCODE -> REQUEST INTERCEPT -> RESPONSE INTERCEPT -> DECODE ] Not sure if I am right, but I can give an example. For eg, my response should be encrypted from the request and decrypted from the response. Then the flow will be: [ENCODE(JSON MAPPING) -> REQUEST INTERCEPT(ENCRYPTED) -> RESPONSE INTERCEPT (DECRYPTED) -> DECODE(JSON MAPPING) ]

As the current flow, the decode is being called before the response intercept. Then I need to put too much logic on the Decode converter to handle the decryption, validate if the response is correct or not before decryption, etc... but It should only handle the JSON converter.

How do u think about it?

lejard-h commented 5 years ago

I am not sure what you are trying to accomplish but I will try to explain.

On the response side I don't see any issue.

the current flow is

httpResponse decode json (optional) custom decode (optional) intercept response

From my point of view it is better to intercept the decoded response and the all "decode" phase is optional so you still can manipulate raw response inside interceptors and do what you want.

However I maybe have an issue on the request side. A request interceptor should be able to change the request before we encode the body and this is not the case here.

edwardaux commented 5 years ago

For what it is worth, I see an interceptor as something that has an opportunity to manipulate the raw request just before it is sent, and/or manipulate the response immediately after it is received.

Perhaps we could consider modifying the interceptors' lifecycle to be something like:

Along these same lines, one thing I did notice when I was debugging my own interceptor is that we grab http.BaseRequest.body (which is a String) and put that into the chopper Request object. For most JSON APIs, this is totally fine... but I do feel that it could cause problems in the future for APIs that return binary data (eg. fetching an image). I feel like there are two different sub-layers: 1) the raw bytes/http layer, 2) the encoded/decoded domain model object layer... and it would be awesome if the interceptors were able to distinguish between those two layers.

Of course, my idea of how the interceptors should work may not be the same as everyone elses 😄

ericta commented 5 years ago

I am not sure what you are trying to accomplish but I will try to explain.

On the response side I don't see any issue.

the current flow is

httpResponse decode json (optional) custom decode (optional) intercept response

From my point of view it is better to intercept the decoded response and the all "decode" phase is optional so you still can manipulate raw response inside interceptors and do what you want.

However I maybe have an issue on the request side. A request interceptor should be able to change the request before we encode the body and this is not the case here.

Just give you the example, that I made a Response Log Interceptor to log the response. However when the Response is coming, it will go through the Convertor first, and for some reason, it is getting the exception! then my ResponseLogInterceptor will be never called! but this one should be call first, on my case!

lejard-h commented 5 years ago

@ericta what exception are you getting ?

except if you have an exception in your converter, it will always go through your response interceptor.

@edwardaux body is dynamic, so you should be able to pass any type you want, but chopper will handle String, List and Map<String, String> ( => https://github.com/lejard-h/chopper/blob/dev/chopper/lib/src/request.dart#L122)

I think that interceptors should only manipulate Dart object coming from you domain model object layer and Converter should transform the raw bytes/http or String to this object layer.

It is not well documented but you have 3 streams that you can listen to.

// just before sending request after all converters and interceptors
Stream<Request> get onRequest => _requestController.stream;

// if response is successfull (statusCode: 200>= <300)
Stream<Response> get onResponse => _responseController.stream;

// if not successful
Stream<Response> get onError => _responseErrorController.stream;

this issue https://github.com/lejard-h/chopper/issues/10 is also linked. on 1.2.0-dev1 you can now directly call request.toHttpRequest() to get the http.BaseRequest that will be send

edwardaux commented 5 years ago

I think that interceptors should only manipulate Dart object coming from you domain model object layer and Converter should transform the raw bytes/http or String to this object layer.

Ah, I think this is the subtlety that I was missing. Thanks for clarifying 👍

lejard-h commented 5 years ago

2.0.0 released