Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

App is crashing when network call returns HTTP 400. Am I implementing this incorrectly? #659

Closed
martymiller opened this issue Oct 26, 2022 · 3 comments

Comments

@martymiller
Copy link

martymiller commented Oct 26, 2022

I'm hoping someone can help me, because I'm totally stuck. I'm trying to capture network errors, so I'm making a network call that I know will fail. Successful calls work fine.

I'm on Mavericks 3.0 and making a call inside my MavericksViewModel. Here is my state class:

Screen Shot 2022-10-26 at 9 40 00 AM

Retrofit service class call:
Screen Shot 2022-10-26 at 9 48 34 AM

Making the call inside a regular (non-suspend) function. I tried putting this inside of a suspend {} like in the docs but the call never gets made. I set a breakpoint inside and it never lands there (don't know why).
Screen Shot 2022-10-26 at 9 50 11 AM

Observing the response:
Screen Shot 2022-10-26 at 9 50 29 AM

Resulting error:
Screen Shot 2022-10-26 at 9 49 21 AM

After the call is made, my authResponse doesn't get set to LOADING or anything. The app immediately crashes. I don't understand why it doesn't fall into the onFail callback. Is this not a FAIL? If it's not then what is a FAIL when it comes to Async? I don't want to have to put every call I make into a try / catch block. So is there something wrong with my implementation?

@elihart
Copy link
Contributor

elihart commented Oct 26, 2022

I recommend looking at the implementation of Flow.execute

    protected open fun <T> Flow<T>.execute(
        dispatcher: CoroutineDispatcher? = null,
        retainValue: KProperty1<S, Async<T>>? = null,
        reducer: S.(Async<T>) -> S
    ): Job {
        val blockExecutions = config.onExecute(this@MavericksViewModel)
        if (blockExecutions != MavericksViewModelConfig.BlockExecutions.No) {
            if (blockExecutions == MavericksViewModelConfig.BlockExecutions.WithLoading) {
                setState { reducer(Loading(value = retainValue?.get(this)?.invoke())) }
            }
            // Simulate infinite loading
            return viewModelScope.launch { delay(Long.MAX_VALUE) }
        }

        setState { reducer(Loading(value = retainValue?.get(this)?.invoke())) }

        return catch { error -> setState { reducer(Fail(error, value = retainValue?.get(this)?.invoke())) } }
            .onEach { value -> setState { reducer(Success(value)) } }
            .launchIn(viewModelScope + (dispatcher ?: EmptyCoroutineContext))
    }

Note that this launches the flow in the viewModelScope, so if you actually want to use IO dispatcher you need to specify that as an argument to execute. Also, it ideally wouldn't be needed to wrap the execute call in a launch call because of this, so ideally you would create your Flow without a suspend context (although that shouldn't really matter).

You can see that execute immediately does setState { reducer(Loading(value = retainValue?.get(this)?.invoke())) } which should set your loading state and then uses catch { error -> setState { reducer(Fail(error, value = retainValue?.get(this)?.invoke())) } } to catch errors and set fail states.

Based on the crash that is happening for you i'm guessing it is thrown from elsewhere, not in this flow, perhaps a mechanism of how retrofit creates its flows. I would recommend stepping through where the crash originates and looking at why it isn't part of the flow collection. It seems specific to retrofit though.

@martymiller
Copy link
Author

martymiller commented Oct 26, 2022

Thanks @elihart I'll look into that. I did try some other things, like making my service function non-suspend and then calling execute(IO), but then I get this error:

Screen Shot 2022-10-26 at 12 24 43 PM

I'm not sure what Retrofit changes I will need to make. Aren't most people using Retrofit at this point? Seems like this would be a common issue.

Also, the API I'm using returns something like this whenever there is an error instead of the expected json"

{
      error: "You entered an incorrect password. Please try again."
      code: "400"
}

I'm trying to figure out how to capture that message even though the response type is different. If you have any ideas, that would be great. Thanks for the help!

@elihart
Copy link
Contributor

elihart commented Oct 27, 2022

@martymiller can you share the solution?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants