jhomlala / alice

HTTP Inspector for Flutter. Allows checking HTTP connections with UI inspector.
544 stars 246 forks source link

chore: refactor `AliceChopperAdapter` #191

Closed techouse closed 3 months ago

techouse commented 3 months ago

This PR refactors the Alice Chopper interceptor even further by:

jhomlala commented 3 months ago

Hey, thanks for this refactor. When I run your adapter with alice_chopper example, it shows me this error. I think this is something with getRequestHashCode. Can you check it please?

I/flutter ( 9961): Bad state: No element
I/flutter ( 9961): #0      Iterable.reduce (dart:core/iterable.dart:375:7)
I/flutter ( 9961): #1      AliceChopperAdapter.getRequestHashCode (package:alice_chopper/alice_chopper_adapter.dart:21:14)
I/flutter ( 9961): #2      AliceChopperAdapter.intercept (package:alice_chopper/alice_chopper_adapter.dart:37:23)
I/flutter ( 9961): <asynchronous suspension>
I/flutter ( 9961): #3      InterceptorChain.proceed (package:chopper/src/chain/interceptor_chain.dart:46:16)
I/flutter ( 9961): <asynchronous suspension>
I/flutter ( 9961): #4      RequestConverterInterceptor.intercept (package:chopper/src/interceptors/request_converter_interceptor.dart:26:7)
I/flutter ( 9961): <asynchronous suspension>
I/flutter ( 9961): #5      InterceptorChain.proceed (package:chopper/src/chain/interceptor_chain.dart:46:16)
I/flutter ( 9961): <asynchronous suspension>
I/flutter ( 9961): #6      Call.execute (package:chopper/src/chain/call.dart:56:12)
I/flutter ( 9961): <asynchronous suspension>
I/flutter ( 9961): #7      ChopperClient.send (package:chopper/src/base.dart:175:22)
I/flutter ( 9961): <asynchronous suspension>
techouse commented 3 months ago

Oops ... lemme check 🙈

jhomlala commented 3 months ago

Maybe I was wrong and alice_token is required :)

techouse commented 3 months ago

Fixed.

Since alice_token was previously always present map + reduce worked. After I removed it and the headers Map became empty this would throw an error, as is explained here.

To work around this I used fold with an initialValue of 0 instead.

jhomlala commented 3 months ago

Amazing! It's not crashing anymore. But when I look into inspector, it looks like responses are not being added into call.

image image

Could you please verify why it's happening? Also I see that Alice should have some unit tests. This is something for next PRs.

techouse commented 3 months ago

I fixed it. Can you please check it again?

jhomlala commented 3 months ago

image Great! Some requests are still loading forever. Could you look again at it when you have some time? 👀

techouse commented 3 months ago

Working on it. I pushed an update that fixed it, but has some analysis issues. I'll check again in the evening.

jhomlala commented 3 months ago

No worries, take your time. Thanks!

techouse commented 3 months ago

@jhomlala right, this should now work as expected.

It turns out that the alice_token is actually required to keep track of all the various AliceHttpCalls and AliceHttpResponses in the AliceCore, so I used UUIDv4 to generate it using https://pub.dev/packages/uuid. This does add another dependency and if this is not desired, then I can revert to using microseconds, i.e. DateTime.now(). microsecondsSinceEpoch.toString().

I've also done extensive work on the Alice + Chopper example.

CC / @Guldem

jhomlala commented 3 months ago

@techouse Thanks for this amazing work! Yes, I think this UUID package is fine. Overall it's LGTM. Are you planning to add more refactors or are we ready to merge it?

techouse commented 3 months ago

Are you planning to add more refactors or are we ready to merge it?

Nope. That's all she wrote. 🤣

jhomlala commented 3 months ago

Included in 1.0.3 version of alice_chopper.