-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[netatmo] API limit reached handling #16489
base: main
Are you sure you want to change the base?
Changes from all commits
86a0837
6048eef
26d3c74
4d87b7f
3242aa7
4870e57
a189039
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -103,6 +103,7 @@ | |
@NonNullByDefault | ||
public class ApiBridgeHandler extends BaseBridgeHandler { | ||
private static final int TIMEOUT_S = 20; | ||
private static final int API_LIMIT_INTERVAL_S = 3600; | ||
|
||
private final Logger logger = LoggerFactory.getLogger(ApiBridgeHandler.class); | ||
private final AuthenticationApi connectApi = new AuthenticationApi(this); | ||
|
@@ -210,8 +211,7 @@ private boolean authenticate(@Nullable String code, @Nullable String redirectUri | |
startAuthorizationFlow(); | ||
return false; | ||
} catch (IOException e) { | ||
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage()); | ||
prepareReconnection(code, redirectUri); | ||
prepareReconnection(getConfiguration().reconnectInterval, e.getMessage(), code, redirectUri); | ||
return false; | ||
} | ||
|
||
|
@@ -239,11 +239,13 @@ public ApiHandlerConfiguration getConfiguration() { | |
return getConfigAs(ApiHandlerConfiguration.class); | ||
} | ||
|
||
private void prepareReconnection(@Nullable String code, @Nullable String redirectUri) { | ||
private void prepareReconnection(int delay, @Nullable String message, @Nullable String code, | ||
@Nullable String redirectUri) { | ||
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, message); | ||
connectApi.dispose(); | ||
freeConnectJob(); | ||
connectJob = Optional.of(scheduler.schedule(() -> openConnection(code, redirectUri), | ||
getConfiguration().reconnectInterval, TimeUnit.SECONDS)); | ||
connectJob = Optional.of(scheduler.schedule(() -> openConnection(code, redirectUri), delay, TimeUnit.SECONDS)); | ||
logger.debug("Reconnection scheduled in {} seconds", delay); | ||
} | ||
|
||
private void freeConnectJob() { | ||
|
@@ -307,7 +309,7 @@ public synchronized <T> T executeUri(URI uri, HttpMethod method, Class<T> clazz, | |
Request request = httpClient.newRequest(uri).method(method).timeout(TIMEOUT_S, TimeUnit.SECONDS); | ||
|
||
if (!authenticate(null, null)) { | ||
prepareReconnection(null, null); | ||
prepareReconnection(getConfiguration().reconnectInterval, null, null, "@text/status-bridge-offline"); | ||
throw new NetatmoException("Not authenticated"); | ||
} | ||
connectApi.getAuthorization().ifPresent(auth -> request.header(HttpHeader.AUTHORIZATION, auth)); | ||
|
@@ -348,27 +350,26 @@ public synchronized <T> T executeUri(URI uri, HttpMethod method, Class<T> clazz, | |
try { | ||
exception = new NetatmoException(deserializer.deserialize(ApiError.class, responseBody)); | ||
} catch (NetatmoException e) { | ||
exception = new NetatmoException("Error deserializing error: %s".formatted(statusCode.getMessage())); | ||
if (statusCode == Code.TOO_MANY_REQUESTS) { | ||
exception = new NetatmoException(statusCode.getMessage()); | ||
} else { | ||
exception = new NetatmoException( | ||
"Error deserializing error: %s".formatted(statusCode.getMessage())); | ||
} | ||
} | ||
throw exception; | ||
} catch (NetatmoException e) { | ||
if (e.getStatusCode() == ServiceError.MAXIMUM_USAGE_REACHED) { | ||
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage()); | ||
prepareReconnection(null, null); | ||
if (statusCode == Code.TOO_MANY_REQUESTS | ||
|| exception.getStatusCode() == ServiceError.MAXIMUM_USAGE_REACHED) { | ||
prepareReconnection(API_LIMIT_INTERVAL_S, | ||
clinique marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"@text/maximum-usage-reached [ \"%d\" ]".formatted(API_LIMIT_INTERVAL_S), null, null); | ||
} | ||
throw e; | ||
} catch (InterruptedException e) { | ||
Thread.currentThread().interrupt(); | ||
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage()); | ||
throw new NetatmoException("Request interrupted"); | ||
} catch (TimeoutException | ExecutionException e) { | ||
throw exception; | ||
} catch (InterruptedException | TimeoutException | ExecutionException e) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure the By initiating a retry below, it seems we are delaying the thread interruption. Perhaps it would be better to propagate the exception? @wborn - can you advise a bit here? |
||
if (retryCount > 0) { | ||
logger.debug("Request timedout, retry counter: {}", retryCount); | ||
logger.debug("Request error, retry counter: {}", retryCount); | ||
return executeUri(uri, method, clazz, payload, contentType, retryCount - 1); | ||
} | ||
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "@text/request-time-out"); | ||
prepareReconnection(null, null); | ||
throw new NetatmoException(String.format("%s: \"%s\"", e.getClass().getName(), e.getMessage())); | ||
prepareReconnection(getConfiguration().reconnectInterval, "@text/request-time-out", null, e.getMessage()); | ||
throw new NetatmoException("%s: \"%s\"".formatted(e.getClass().getName(), e.getMessage())); | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Status is now set twice when authentication fails - see #16489 (comment)
It's set by
startAuthorizationFlow
, and then again here when called from line 312.