Skip to content
This repository has been archived by the owner on Feb 23, 2022. It is now read-only.

Enhance exception occurred in reducer() w/ state and action #13

Conversation

artem-zinnatullin
Copy link
Contributor

PR enhances error information with current state and action if an error happens inside reducer()'s logic in the user code.

Motivation: it lets the user easily reproduce crashes, reducer() can be just called with this state and action combination that created a crash.


Tech details:

I went with introducing ReducerException, but I'm not super opinionated on that, if you think that we can use some existing exception type, I'm fine with that. I do however like domain-specific exceptions instead of generic ones.

Also, I had to add : Any constraints on state and action generics because otherwise Kotlin compiler didn't let me assign them to Any fields (which sounds like a compiler bug, right?). And Exceptions are not allowed to be parametrized.

@sockeqwe
Copy link
Contributor

Thanks! Yeah, makes sense to add ReducerException 👍
I'm just not sure exposing state and action could leak private data like user credentials or password. You are the (paranoid 😸 ) security expert, any thoughts?
Adding Any sounds indeed strange but I have the feeling this was done on purpose by the compiler team. I think I saw some discussion around this at YouTrack but I might be wrong or confusing this with something else.

I was wondering if adding generics to ReducerException<S, A> is a good idea. Not sure if people really implement onError() in the subscription (or some other error handlers downstream) but then they should know which type of state and which type of action is used in this Rx Stream, so that they can consume this exception easily by casting I guess.

@artem-zinnatullin
Copy link
Contributor Author

I'm just not sure exposing state and action could leak private data like user credentials or password. You are the (paranoid 😸 ) security expert, any thoughts?

data classes that contain confidential data should have custom toString() anyway, no matter if they use RxRedux or not! (there are helper projects like https://github.com/hzsweers/redacted-compiler-plugin to do that)

@sockeqwe
Copy link
Contributor

sockeqwe commented Oct 2, 2018

Your are right 👍
Merging this in the afternoon (will set up CI for auto release new versions to maven central from CI before)

@sockeqwe sockeqwe merged commit 0d98388 into freeletics:master Oct 3, 2018
@artem-zinnatullin artem-zinnatullin deleted the az/add-state-and-action-to-exception branch October 5, 2018 04:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants