chore(exception)!: Enhance exception handling
- Replace http://localhost/ with http://example.com[/] because localhost ist a possible valid host and therefore confusing for users. Considered info-removed-for-privacy-reasons.com (or something similar), but there is no guarantee that this domain is not registered. example.com can by definition not be registered. - Unify way to create a response on an exception by passing the whole exception object into the helper method - Use HTTP 900 for unknown exceptions to distinguish other proprietary status codes: HTTP 520 is for example used by Cloudflare. Note that HTTP 900 is also not standardized. Signed-off-by: Stefan Niedermann <info@niedermann.it>
This commit is contained in:
Stefan Niedermann
Andy Scherzinger
Родитель
71a46387e5
Коммит
f6faee86d1
|
|
@ -98,7 +98,7 @@ public class AidlNetworkRequest extends NetworkRequest {
|
|||
try {
|
||||
final Intent intentService = new Intent();
|
||||
intentService.setComponent(new ComponentName(componentName,
|
||||
"com.owncloud.android.services.AccountManagerService"));
|
||||
"com.owncloud.android.services.AccountManagerService"));
|
||||
// https://developer.android.com/reference/android/content/Context#BIND_ABOVE_CLIENT
|
||||
if (!mContext.bindService(intentService, mConnection, Context.BIND_AUTO_CREATE | Context.BIND_ABOVE_CLIENT)) {
|
||||
Log.d(TAG, "[connect] Binding to AccountManagerService returned false");
|
||||
|
|
@ -171,14 +171,14 @@ public class AidlNetworkRequest extends NetworkRequest {
|
|||
final ParcelFileDescriptor output = performAidlNetworkRequestV2(request, requestBodyInputStream);
|
||||
final InputStream os = new ParcelFileDescriptor.AutoCloseInputStream(output);
|
||||
try {
|
||||
final ExceptionResponse response = deserializeObjectV2(os);
|
||||
final var response = deserializeObjectV2(os);
|
||||
|
||||
// Handle Remote Exceptions
|
||||
if (response.getException() != null) {
|
||||
if (response.getException().getMessage() != null) {
|
||||
throw parseNextcloudCustomException(mContext, response.getException());
|
||||
if (response.exception() != null) {
|
||||
if (response.exception().getMessage() != null) {
|
||||
throw parseNextcloudCustomException(mContext, response.exception());
|
||||
}
|
||||
throw response.getException();
|
||||
throw response.exception();
|
||||
}
|
||||
// os stream needs to stay open to be able to read response
|
||||
return new Response(os, response.headers);
|
||||
|
|
@ -192,9 +192,8 @@ public class AidlNetworkRequest extends NetworkRequest {
|
|||
/**
|
||||
* <strong>DO NOT CALL THIS METHOD DIRECTLY</strong> - use {@link #performNetworkRequestV2} instead
|
||||
*
|
||||
* @param request
|
||||
* @return
|
||||
* @throws IOException
|
||||
* @param request, requestBodyInputStream
|
||||
* @throws IOException, RemoteException, NextcloudApiNotRespondingException
|
||||
*/
|
||||
private ParcelFileDescriptor performAidlNetworkRequestV2(@NonNull NextcloudRequest request,
|
||||
@Nullable InputStream requestBodyInputStream)
|
||||
|
|
@ -245,7 +244,7 @@ public class AidlNetworkRequest extends NetworkRequest {
|
|||
}
|
||||
}
|
||||
|
||||
return new ExceptionResponse(exception, headerList);
|
||||
return new ExceptionResponse(headerList, exception);
|
||||
}
|
||||
|
||||
public static class PlainHeader implements Serializable {
|
||||
|
|
@ -276,17 +275,9 @@ public class AidlNetworkRequest extends NetworkRequest {
|
|||
}
|
||||
}
|
||||
|
||||
private static class ExceptionResponse {
|
||||
private final Exception exception;
|
||||
private final ArrayList<PlainHeader> headers;
|
||||
|
||||
public ExceptionResponse(Exception exception, ArrayList<PlainHeader> headers) {
|
||||
this.exception = exception;
|
||||
this.headers = headers;
|
||||
}
|
||||
|
||||
public Exception getException() {
|
||||
return exception;
|
||||
}
|
||||
private record ExceptionResponse(
|
||||
@NonNull ArrayList<PlainHeader> headers,
|
||||
@Nullable Exception exception
|
||||
) {
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -88,23 +88,20 @@ public class SSOException extends Exception {
|
|||
return new UnknownErrorException("Exception message is null");
|
||||
}
|
||||
|
||||
switch (message) {
|
||||
case Constants.EXCEPTION_INVALID_TOKEN:
|
||||
return new TokenMismatchException(context);
|
||||
case Constants.EXCEPTION_ACCOUNT_NOT_FOUND:
|
||||
return new NextcloudFilesAppAccountNotFoundException(context);
|
||||
case Constants.EXCEPTION_UNSUPPORTED_METHOD:
|
||||
return new NextcloudUnsupportedMethodException(context);
|
||||
case Constants.EXCEPTION_INVALID_REQUEST_URL:
|
||||
return new NextcloudInvalidRequestUrlException(context, exception.getCause());
|
||||
case Constants.EXCEPTION_HTTP_REQUEST_FAILED:
|
||||
return switch (message) {
|
||||
case Constants.EXCEPTION_INVALID_TOKEN -> new TokenMismatchException(context);
|
||||
case Constants.EXCEPTION_ACCOUNT_NOT_FOUND -> new NextcloudFilesAppAccountNotFoundException(context);
|
||||
case Constants.EXCEPTION_UNSUPPORTED_METHOD -> new NextcloudUnsupportedMethodException(context);
|
||||
case Constants.EXCEPTION_INVALID_REQUEST_URL ->
|
||||
new NextcloudInvalidRequestUrlException(context, exception.getCause());
|
||||
case Constants.EXCEPTION_HTTP_REQUEST_FAILED -> {
|
||||
final int statusCode = Integer.parseInt(exception.getCause().getMessage());
|
||||
final var cause = exception.getCause().getCause();
|
||||
return new NextcloudHttpRequestFailedException(context, statusCode, cause);
|
||||
case Constants.EXCEPTION_ACCOUNT_ACCESS_DECLINED:
|
||||
return new NextcloudFilesAppAccountPermissionNotGrantedException(context);
|
||||
default:
|
||||
return new UnknownErrorException(exception.getMessage());
|
||||
}
|
||||
yield new NextcloudHttpRequestFailedException(context, statusCode, cause);
|
||||
}
|
||||
case Constants.EXCEPTION_ACCOUNT_ACCESS_DECLINED ->
|
||||
new NextcloudFilesAppAccountPermissionNotGrantedException(context);
|
||||
default -> new UnknownErrorException(exception.getMessage());
|
||||
};
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -13,22 +13,20 @@ package com.nextcloud.android.sso.helper;
|
|||
import androidx.annotation.NonNull;
|
||||
|
||||
import com.nextcloud.android.sso.aidl.NextcloudRequest;
|
||||
import com.nextcloud.android.sso.api.EmptyResponse;
|
||||
import com.nextcloud.android.sso.api.AidlNetworkRequest;
|
||||
import com.nextcloud.android.sso.api.NextcloudAPI;
|
||||
import com.nextcloud.android.sso.exceptions.NextcloudHttpRequestFailedException;
|
||||
|
||||
import java.lang.reflect.Type;
|
||||
import java.util.ArrayList;
|
||||
import java.util.HashMap;
|
||||
import java.util.Map;
|
||||
import java.util.Arrays;
|
||||
import java.util.Collections;
|
||||
import java.util.Optional;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
import okhttp3.Headers;
|
||||
import okhttp3.MediaType;
|
||||
import okhttp3.Protocol;
|
||||
import okhttp3.Request;
|
||||
import okhttp3.ResponseBody;
|
||||
import okio.BufferedSource;
|
||||
import okio.Timeout;
|
||||
import retrofit2.Call;
|
||||
import retrofit2.Callback;
|
||||
|
|
@ -36,9 +34,11 @@ import retrofit2.Response;
|
|||
|
||||
public final class Retrofit2Helper {
|
||||
|
||||
private Retrofit2Helper() { }
|
||||
private Retrofit2Helper() {
|
||||
}
|
||||
|
||||
public static <T> Call<T> WrapInCall(final NextcloudAPI nextcloudAPI, final NextcloudRequest nextcloudRequest,
|
||||
public static <T> Call<T> WrapInCall(final NextcloudAPI nextcloudAPI,
|
||||
final NextcloudRequest nextcloudRequest,
|
||||
final Type resType) {
|
||||
return new Call<>() {
|
||||
|
||||
|
|
@ -49,22 +49,23 @@ public final class Retrofit2Helper {
|
|||
@Override
|
||||
public Response<T> execute() {
|
||||
try {
|
||||
com.nextcloud.android.sso.api.Response response = nextcloudAPI.performNetworkRequestV2(nextcloudRequest);
|
||||
final var response = nextcloudAPI.performNetworkRequestV2(nextcloudRequest);
|
||||
final T body = nextcloudAPI.convertStreamToTargetEntity(response.getBody(), resType);
|
||||
final var headerMap = Optional.ofNullable(response.getPlainHeaders())
|
||||
.map(headers -> headers
|
||||
.stream()
|
||||
.collect(Collectors.toMap(
|
||||
AidlNetworkRequest.PlainHeader::getName,
|
||||
AidlNetworkRequest.PlainHeader::getValue)))
|
||||
.orElse(Collections.emptyMap());
|
||||
|
||||
T body = nextcloudAPI.convertStreamToTargetEntity(response.getBody(), resType);
|
||||
Map<String, String> headerMap = new HashMap<>();
|
||||
ArrayList<AidlNetworkRequest.PlainHeader> plainHeaders = response.getPlainHeaders();
|
||||
if (plainHeaders != null) {
|
||||
for (AidlNetworkRequest.PlainHeader header : plainHeaders) {
|
||||
headerMap.put(header.getName(), header.getValue());
|
||||
}
|
||||
}
|
||||
return Response.success(body, Headers.of(headerMap));
|
||||
|
||||
} catch (NextcloudHttpRequestFailedException e) {
|
||||
final Throwable cause = e.getCause();
|
||||
return convertExceptionToResponse(e.getStatusCode(), cause == null ? e.getMessage() : cause.getMessage());
|
||||
return convertExceptionToResponse(e.getStatusCode(), Optional.ofNullable(e.getCause()).orElse(e));
|
||||
|
||||
} catch (Exception e) {
|
||||
return convertExceptionToResponse(520, e.toString());
|
||||
return convertExceptionToResponse(900, e);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -73,8 +74,7 @@ public final class Retrofit2Helper {
|
|||
*/
|
||||
@Override
|
||||
public void enqueue(@NonNull final Callback<T> callback) {
|
||||
final Call<T> call = this;
|
||||
new Thread(() -> callback.onResponse(call, execute())).start();
|
||||
new Thread(() -> callback.onResponse(this, execute())).start();
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
@ -110,17 +110,19 @@ public final class Retrofit2Helper {
|
|||
throw new UnsupportedOperationException("Not implemented");
|
||||
}
|
||||
|
||||
private Response<T> convertExceptionToResponse(int statusCode, String errorMessage) {
|
||||
ResponseBody body = ResponseBody.create(null, errorMessage);
|
||||
private Response<T> convertExceptionToResponse(int statusCode, @NonNull Throwable throwable) {
|
||||
final var body = ResponseBody.create(null, throwable.toString());
|
||||
final var path = Optional.ofNullable(nextcloudRequest.getUrl()).orElse("");
|
||||
|
||||
return Response.error(
|
||||
body,
|
||||
new okhttp3.Response.Builder()
|
||||
.body(body)
|
||||
.code(statusCode)
|
||||
.message(errorMessage)
|
||||
.protocol(Protocol.HTTP_1_1)
|
||||
.request(new Request.Builder().url("http://localhost/" + nextcloudRequest.getUrl()).build())
|
||||
.build());
|
||||
body,
|
||||
new okhttp3.Response.Builder()
|
||||
.body(body)
|
||||
.code(statusCode)
|
||||
.message(throwable.getMessage() + Arrays.toString(throwable.getStackTrace()))
|
||||
.protocol(Protocol.HTTP_1_1)
|
||||
.request(new Request.Builder().url("http://example.com" + (path.startsWith("/") ? path : "/" + path)).build())
|
||||
.build());
|
||||
}
|
||||
};
|
||||
}
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче