Skip to content

Commit

Permalink
HttpCacheMissException masks ApolloException in case of NETWORK_FIRST (
Browse files Browse the repository at this point in the history
…#3957)

* HttpCacheMissException masks ApolloException in case of NETWORK_FIRST cache policy

* Refactor ApolloCompositeException

1. Use Throwable.addSuppressed to ensure both stacktrace are thrown
2. Annotate with @throws to make sure client is aware of RuntimeExceptions

* Replace with ApolloCompositeException in respect of code consistency

* Update apollo-http-cache/src/main/kotlin/com/apollographql/apollo3/cache/http/CachingHttpInterceptor.kt

Co-authored-by: Martin Bonnin <martin@mbonnin.net>

* Simplify catch block suggested by @martinbonnin

* For backward compatibility keep cause to secondException itself

* @throws is only available for jvm packages

* Reverted the parameter name changes

* fix wrong calling argument name used

* comments updated for ApolloCompositeException and NETWORK_FIRST fetch policy

* Add both exceptions in ApolloCompositeExceptions as suppressed

Also deprecate first and second exceptions usage

* split if conditions

Co-authored-by: Martin Bonnin <martin@mbonnin.net>
  • Loading branch information
akshay253101 and martinbonnin authored Mar 25, 2022
1 parent 9bd26f4 commit 6853ae8
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,30 @@ class CacheMissException(val key: String, val fieldName: String? = null) : Apoll
class HttpCacheMissException(message: String, cause: Exception? = null) : ApolloException(message = message, cause = cause)

/**
* Multiple exceptions happened, for an exemple with a [CacheFirst] fetch policy
* Multiple exceptions happened, for an example with a [CacheFirst] fetch policy
* [cause] might change in future where both exceptions will be added as suppressed exception
*/
class ApolloCompositeException(first: Throwable?, second: Throwable?) : ApolloException(message = "multiple exceptions happened", second) {
val first = (first as? ApolloException) ?: throw RuntimeException("unexpected first exception", first)
val second = (second as? ApolloException) ?: throw RuntimeException("unexpected second exception", second)

@get:Deprecated("Use suppressedExceptions instead", ReplaceWith("suppressedExceptions.first()"))
val first: ApolloException
get() {
val firstException = suppressedExceptions.firstOrNull()
return (firstException as? ApolloException) ?: throw RuntimeException("unexpected first exception", firstException)
}

@get:Deprecated("Use suppressedExceptions instead", ReplaceWith("suppressedExceptions.getOrNull(1)"))
val second: ApolloException
get() {
val secondException = suppressedExceptions.getOrNull(1)
return (secondException as? ApolloException) ?: throw RuntimeException("unexpected second exception", secondException)
}

init {
if (first != null) addSuppressed(first)
if (second != null) addSuppressed(second)
}

}

class AutoPersistedQueriesNotSupported : ApolloException(message = "The server does not support auto persisted queries")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import com.apollographql.apollo3.api.http.HttpMethod
import com.apollographql.apollo3.api.http.HttpRequest
import com.apollographql.apollo3.api.http.HttpResponse
import com.apollographql.apollo3.api.http.valueOf
import com.apollographql.apollo3.exception.ApolloCompositeException
import com.apollographql.apollo3.exception.ApolloException
import com.apollographql.apollo3.exception.ApolloHttpException
import com.apollographql.apollo3.exception.HttpCacheMissException
import com.apollographql.apollo3.network.http.HttpInterceptor
import com.apollographql.apollo3.network.http.HttpInterceptorChain
Expand Down Expand Up @@ -48,17 +50,36 @@ class CachingHttpInterceptor(
return networkMightThrow(request, chain, cacheKey)
}
NETWORK_FIRST -> {
val networkException: ApolloException
try {
val response = networkMightThrow(request, chain, cacheKey)
if (response.statusCode in 200..299) {
// let HTTP errors through
return response
} else {
throw ApolloHttpException(
statusCode = response.statusCode,
headers = response.headers,
body = null,
message = "Http request failed with status code `${response.statusCode}`"
)
}
} catch (e: ApolloException) {

// Original cause of network request failure
networkException = e
}

return cacheMightThrow(request, cacheKey)
try {
return cacheMightThrow(request, cacheKey)
} catch (cacheMissException: HttpCacheMissException) {
// In case of exception thrown by network request,
// ApolloException will be suppressed and HttpCacheMissException will throw as cause
// this behavior might change in future where both will be treated as suppressed
throw ApolloCompositeException(
first = networkException,
second = cacheMissException
)
}
}
else -> {
error("Unknown HTTP fetch policy: $policy")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,8 @@ fun <D : Query.Data> ApolloCall<D>.executeCacheAndNetwork(): Flow<ApolloResponse

if (cacheException != null && networkException != null) {
throw ApolloCompositeException(
cacheException,
networkException
first = cacheException,
second = networkException
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ val CacheFirstInterceptor = object : ApolloInterceptor {
}

throw ApolloCompositeException(
cacheException,
networkException
first = cacheException,
second = networkException
)
}
}
Expand Down Expand Up @@ -145,8 +145,8 @@ val NetworkFirstInterceptor = object : ApolloInterceptor {
}

throw ApolloCompositeException(
networkException,
cacheException,
first = networkException,
second = cacheException,
)
}
}
Expand Down Expand Up @@ -203,8 +203,8 @@ val CacheAndNetworkInterceptor = object : ApolloInterceptor {
}
if (cacheException != null) {
throw ApolloCompositeException(
cacheException,
networkException
first = cacheException,
second = networkException
)
}
throw networkException!!
Expand Down

0 comments on commit 6853ae8

Please sign in to comment.