Skip to content

Commit

Permalink
[http] enhance error handling and add tests (#84)
Browse files Browse the repository at this point in the history
* enable thing status changes on request result
* add lastSuccess and lastFailure channels
* enable strict error handling
* tests

Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
  • Loading branch information
J-N-K committed Apr 29, 2021
1 parent 3c0ad44 commit fd20d27
Show file tree
Hide file tree
Showing 14 changed files with 778 additions and 101 deletions.
8 changes: 6 additions & 2 deletions bundles/org.smarthomej.binding.http/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ It can be extended with different channels.
| `contentType` | yes | - | MIME content-type of the command requests. Only used for `PUT` and `POST`. |
| `encoding` | yes | - | Encoding to be used if no encoding is found in responses (advanced parameter). |
| `headers` | yes | - | Additional headers that are sent along with the request. Format is "header=value". Multiple values can be stored as `headers="key1=value1", "key2=value2", "key3=value3",`|
| `ignoreSSLErrors` | no | false | If set to true ignores invalid SSL certificate errors. This is potentially dangerous.|
| `userAgent` | yes | yes | Sets a custom user agent (default is "Jetty/version", e.g. "Jetty/9.4.20.v20190813"). |
| `ignoreSSLErrors` | no | false | If set to true, ignores invalid SSL certificate errors. This is potentially dangerous.|
| `strictErrorHandling` | no | false | If set to true, thing status is changed depending on last request result (failed = `OFFLINE`). Failed requests result in `UNDEF` for channel values. |
| `userAgent` | yes | (yes ) | Sets a custom user agent (default is "Jetty/version", e.g. "Jetty/9.4.20.v20190813"). |

*Note:* Optional "no" means that you have to configure a value unless a default is provided, and you are ok with that setting.

Expand All @@ -42,6 +43,9 @@ Using escaped strings in URL parameters may lead to problems with the formatting

## Channels

The thing has two channels of type `requestDateTime` which provide the timestamp of the last successful (`lastSuccess`) and last failed (`lastFailure`) request.

Additionally, the thing can be extended with data channels.
Each item type has its own channel-type.
Depending on the channel-type, channels have different configuration options.
All channel-types (except `image`) have `stateExtension`, `commandExtension`, `stateTransformation`, `commandTransformation` and `mode` parameters.
Expand Down
64 changes: 64 additions & 0 deletions bundles/org.smarthomej.binding.http/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,77 @@

<name>SmartHome/J Add-ons :: Bundles :: HTTP Binding</name>

<properties>
<jetty.version>9.4.20.v20190813</jetty.version>
</properties>

<dependencies>
<dependency>
<groupId>org.smarthomej.addons.bundles</groupId>
<artifactId>org.smarthomej.commons</artifactId>
<version>${project.version}</version>
<scope>provided</scope>
</dependency>

<!-- testing, we need to exclude and declare jetty bundles because the declared transitive dependency 9.2.28 is too old -->
<dependency>
<groupId>com.github.tomakehurst</groupId>
<artifactId>wiremock</artifactId>
<version>2.27.2</version>
<scope>test</scope>
<exclusions>
<exclusion>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-server</artifactId>
</exclusion>
<exclusion>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-servlet</artifactId>
</exclusion>
<exclusion>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-servlets</artifactId>
</exclusion>
<exclusion>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-proxy</artifactId>
</exclusion>
<exclusion>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-webapp</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-server</artifactId>
<version>${jetty.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-servlet</artifactId>
<version>${jetty.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-servlets</artifactId>
<version>${jetty.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-proxy</artifactId>
<version>${jetty.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-webapp</artifactId>
<version>${jetty.version}</version>
<scope>test</scope>
</dependency>
</dependencies>

</project>
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.openhab.core.thing.ThingTypeUID;
import org.openhab.core.thing.type.ChannelTypeUID;

/**
* The {@link HttpBindingConstants} class defines common constants, which are
Expand All @@ -24,8 +25,12 @@
*/
@NonNullByDefault
public class HttpBindingConstants {

private static final String BINDING_ID = "http";

public static final ThingTypeUID THING_TYPE_URL = new ThingTypeUID(BINDING_ID, "url");

public static final ChannelTypeUID REQUEST_DATE_TIME_CHANNELTYPE_UID = new ChannelTypeUID(BINDING_ID,
"requestDateTime");
public static final String CHANNEL_LAST_SUCCESS = "lastSuccess";
public static final String CHANNEL_LAST_FAILURE = "lastFailure";
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@
*/
package org.smarthomej.binding.http.internal;

import static org.smarthomej.binding.http.internal.HttpBindingConstants.CHANNEL_LAST_FAILURE;
import static org.smarthomej.binding.http.internal.HttpBindingConstants.CHANNEL_LAST_SUCCESS;
import static org.smarthomej.binding.http.internal.HttpBindingConstants.REQUEST_DATE_TIME_CHANNELTYPE_UID;

import java.net.MalformedURLException;
import java.net.URI;
import java.net.URISyntaxException;
Expand All @@ -28,14 +32,10 @@

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.api.Authentication;
import org.eclipse.jetty.client.api.AuthenticationStore;
import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.client.util.BasicAuthentication;
import org.eclipse.jetty.client.util.DigestAuthentication;
import org.eclipse.jetty.client.util.StringContentProvider;
import org.eclipse.jetty.http.HttpMethod;
import org.openhab.core.library.types.DateTimeType;
import org.openhab.core.library.types.PointType;
import org.openhab.core.library.types.StringType;
Expand All @@ -44,7 +44,6 @@
import org.openhab.core.thing.Thing;
import org.openhab.core.thing.ThingStatus;
import org.openhab.core.thing.ThingStatusDetail;
import org.openhab.core.thing.binding.BaseThingHandler;
import org.openhab.core.types.Command;
import org.openhab.core.types.RefreshType;
import org.openhab.core.types.State;
Expand All @@ -56,9 +55,11 @@
import org.smarthomej.binding.http.internal.config.HttpThingConfig;
import org.smarthomej.binding.http.internal.http.HttpAuthException;
import org.smarthomej.binding.http.internal.http.HttpResponseListener;
import org.smarthomej.binding.http.internal.http.HttpStatusListener;
import org.smarthomej.binding.http.internal.http.RateLimitedHttpClient;
import org.smarthomej.binding.http.internal.http.RefreshingUrlCache;
import org.smarthomej.commons.SimpleDynamicStateDescriptionProvider;
import org.smarthomej.commons.UpdatingBaseThingHandler;
import org.smarthomej.commons.itemvalueconverter.ChannelMode;
import org.smarthomej.commons.itemvalueconverter.ContentWrapper;
import org.smarthomej.commons.itemvalueconverter.ItemValueConverter;
Expand All @@ -80,14 +81,13 @@
* @author Jan N. Klug - Initial contribution
*/
@NonNullByDefault
public class HttpThingHandler extends BaseThingHandler {
public class HttpThingHandler extends UpdatingBaseThingHandler implements HttpStatusListener {
private static final Set<Character> URL_PART_DELIMITER = Set.of('/', '?', '&');

private final Logger logger = LoggerFactory.getLogger(HttpThingHandler.class);
private final ValueTransformationProvider valueTransformationProvider;
private final HttpClientProvider httpClientProvider;
private HttpClient httpClient;
private RateLimitedHttpClient rateLimitedHttpClient;
private final RateLimitedHttpClient rateLimitedHttpClient;
private final SimpleDynamicStateDescriptionProvider httpDynamicStateDescriptionProvider;

private HttpThingConfig config = new HttpThingConfig();
Expand All @@ -100,8 +100,7 @@ public HttpThingHandler(Thing thing, HttpClientProvider httpClientProvider,
SimpleDynamicStateDescriptionProvider httpDynamicStateDescriptionProvider) {
super(thing);
this.httpClientProvider = httpClientProvider;
this.httpClient = httpClientProvider.getSecureClient();
this.rateLimitedHttpClient = new RateLimitedHttpClient(httpClient, scheduler);
this.rateLimitedHttpClient = new RateLimitedHttpClient(httpClientProvider.getSecureClient(), scheduler);
this.valueTransformationProvider = valueTransformationProvider;
this.httpDynamicStateDescriptionProvider = httpDynamicStateDescriptionProvider;
}
Expand All @@ -120,7 +119,10 @@ public void handleCommand(ChannelUID channelUID, Command command) {
RefreshingUrlCache refreshingUrlCache = urlHandlers.get(key);
if (refreshingUrlCache != null) {
try {
refreshingUrlCache.get().ifPresent(itemValueConverter::process);
refreshingUrlCache.get().ifPresentOrElse(itemValueConverter::process, () -> {
if (config.strictErrorHandling)
itemValueConverter.process(null);
});
} catch (IllegalArgumentException | IllegalStateException e) {
logger.warn("Failed processing REFRESH command for channel {}: {}", channelUID, e.getMessage());
}
Expand Down Expand Up @@ -157,12 +159,11 @@ public void initialize() {
// check SSL handling and initialize client
if (config.ignoreSSLErrors) {
logger.info("Using the insecure client for thing '{}'.", thing.getUID());
httpClient = httpClientProvider.getInsecureClient();
rateLimitedHttpClient.setHttpClient(httpClientProvider.getInsecureClient());
} else {
logger.info("Using the secure client for thing '{}'.", thing.getUID());
httpClient = httpClientProvider.getSecureClient();
rateLimitedHttpClient.setHttpClient(httpClientProvider.getSecureClient());
}
rateLimitedHttpClient.setHttpClient(httpClient);
rateLimitedHttpClient.setDelay(config.delay);

int channelCount = thing.getChannels().size();
Expand All @@ -180,7 +181,7 @@ public void initialize() {
// configure authentication
if (!config.username.isEmpty() || !config.password.isEmpty()) {
try {
AuthenticationStore authStore = httpClient.getAuthenticationStore();
AuthenticationStore authStore = rateLimitedHttpClient.getAuthenticationStore();
URI uri = new URI(config.baseURL);
switch (config.authMode) {
case BASIC_PREEMPTIVE:
Expand Down Expand Up @@ -222,7 +223,7 @@ public void initialize() {
// create channels
thing.getChannels().forEach(this::createChannel);

updateStatus(ThingStatus.ONLINE);
updateStatus(ThingStatus.UNKNOWN);
}

@Override
Expand All @@ -248,6 +249,10 @@ public void dispose() {
* @param channel a thing channel
*/
private void createChannel(Channel channel) {
if (REQUEST_DATE_TIME_CHANNELTYPE_UID.equals(channel.getChannelTypeUID())) {
// do not generate refreshUrls for lastSuccess / lastFailure channels
return;
}
ChannelUID channelUID = channel.getUID();
HttpChannelConfig channelConfig = channel.getConfiguration().as(HttpChannelConfig.class);

Expand Down Expand Up @@ -311,8 +316,11 @@ private void createChannel(Channel channel) {
// we need a key consisting of stateContent and URL, only if both are equal, we can use the same cache
String key = channelConfig.stateContent + "$" + stateUrl;
channelUrls.put(channelUID, key);
Objects.requireNonNull(urlHandlers.computeIfAbsent(key, k -> new RefreshingUrlCache(scheduler,
rateLimitedHttpClient, stateUrl, config, channelConfig.stateContent)))
Objects.requireNonNull(
urlHandlers
.computeIfAbsent(key,
k -> new RefreshingUrlCache(scheduler, rateLimitedHttpClient, stateUrl, config,
channelConfig.stateContent, this)))
.addConsumer(itemValueConverter::process);
}

Expand All @@ -324,6 +332,21 @@ private void createChannel(Channel channel) {
}
}

@Override
public void onHttpError(@Nullable String message) {
updateState(CHANNEL_LAST_FAILURE, new DateTimeType());
if (config.strictErrorHandling) {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
Objects.requireNonNullElse(message, ""));
}
}

@Override
public void onHttpSuccess() {
updateState(CHANNEL_LAST_SUCCESS, new DateTimeType());
updateStatus(ThingStatus.ONLINE);
}

private void sendHttpValue(String commandUrl, String command) {
sendHttpValue(commandUrl, command, false);
}
Expand All @@ -334,43 +357,32 @@ private void sendHttpValue(String commandUrl, String command, boolean isRetry) {
URI uri = Util.uriFromString(String.format(commandUrl, new Date(), command));

// build request
Request request = httpClient.newRequest(uri).timeout(config.timeout, TimeUnit.MILLISECONDS)
.method(config.commandMethod);
if (config.commandMethod != HttpMethod.GET) {
final String contentType = config.contentType;
if (contentType != null) {
request.content(new StringContentProvider(command), contentType);
} else {
request.content(new StringContentProvider(command));
}
}

config.getHeaders().forEach(request::header);

if (logger.isTraceEnabled()) {
logger.trace("Sending to '{}': {}", uri, Util.requestToLogString(request));
}

CompletableFuture<@Nullable ContentWrapper> f = new CompletableFuture<>();
f.exceptionally(e -> {
if (e instanceof HttpAuthException) {
if (isRetry) {
logger.warn("Retry after authentication failure failed again for '{}', failing here", uri);
} else {
AuthenticationStore authStore = httpClient.getAuthenticationStore();
Authentication.Result authResult = authStore.findAuthenticationResult(uri);
if (authResult != null) {
authStore.removeAuthenticationResult(authResult);
logger.debug("Cleared authentication result for '{}', retrying immediately", uri);
sendHttpValue(commandUrl, command, true);
} else {
logger.warn("Could not find authentication result for '{}', failing here", uri);
rateLimitedHttpClient.newPriorityRequest(uri, config.commandMethod, command, config.contentType)
.thenAccept(request -> {
request.timeout(config.timeout, TimeUnit.MILLISECONDS);
config.getHeaders().forEach(request::header);

CompletableFuture<@Nullable ContentWrapper> responseContentFuture = new CompletableFuture<>();
responseContentFuture.exceptionally(t -> {
if (t instanceof HttpAuthException) {
if (isRetry || !rateLimitedHttpClient.reAuth(uri)) {
logger.warn(
"Retry after authentication failure failed again for '{}', failing here",
uri);
onHttpError("Authorization failed");
} else {
sendHttpValue(commandUrl, command, true);
}
}
return null;
});

if (logger.isTraceEnabled()) {
logger.trace("Sending to '{}': {}", uri, Util.requestToLogString(request));
}
}
}
return null;
});
request.send(new HttpResponseListener(f, null, config.bufferSize));

request.send(new HttpResponseListener(responseContentFuture, null, config.bufferSize, this));
});
} catch (IllegalArgumentException | URISyntaxException | MalformedURLException e) {
logger.warn("Creating request for '{}' failed: {}", commandUrl, e.getMessage());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public class HttpThingConfig {
public @Nullable String contentType = null;

public boolean ignoreSSLErrors = false;
public boolean strictErrorHandling = false;

// ArrayList is required as implementation because list may be modified later
public ArrayList<String> headers = new ArrayList<>();
Expand Down
Loading

0 comments on commit fd20d27

Please sign in to comment.