twegener-embertec
commented
3 years ago Relevant code paths:
src/tsd/HttpSerializer.java
public ChannelBuffer formatErrorV1(final BadRequestException exception) {
StringBuilder output =
new StringBuilder(exception.getMessage().length() * 2);src/tsd/HttpQuery.java
public void badRequest(final BadRequestException exception) {
logWarn("Bad Request on " + request().getUri() + ": " + exception.getMessage());
if (this.api_version > 0) {
// always default to the latest version of the error formatter since we
// need to return something
switch (this.api_version) {
case 1:
default:
sendReply(exception.getStatus(), serializer.formatErrorV1(exception));
}That warning log message appeared, and exception.getMessage() was stringified as "null", because exception.getMessage() is null.
Here is the proposed code path that led to this getting hit in my particular case:
src/tsd/RpcHandler.java
private void handleHttpQuery(final TSDB tsdb, final Channel chan, final HttpRequest req) {
AbstractHttpQuery abstractQuery = null;
try {
abstractQuery = createQueryInstance(tsdb, req, chan);
...
} catch (BadRequestException ex) {
if (abstractQuery == null) {
LOG.warn("{} Unable to create query for {}. Reason: {}", chan, req, ex);
sendStatusAndClose(chan, HttpResponseStatus.BAD_REQUEST);
} else {
abstractQuery.badRequest(ex);
}src/tsd/QueryRpc.java
public void execute(final TSDB tsdb, final HttpQuery query)
...
final String[] uri = query.explodeAPIPath();
final String endpoint = uri.length > 1 ? uri[1] : "";
if (endpoint.toLowerCase().equals("last")) {
handleLastDataPointQuery(tsdb, query);
...
private void handleLastDataPointQuery(final TSDB tsdb, final HttpQuery query) {
...
final class ErrBack implements Callback<Object, Exception> {
public Object call(final Exception e) throws Exception {
Throwable ex = e;
while (ex.getClass().equals(DeferredGroupException.class)) {
if (ex.getCause() == null) {
LOG.warn("Unable to get to the root cause of the DGE");
break;
}
ex = ex.getCause();
}
if (ex instanceof RuntimeException) {
throw new BadRequestException(ex);
...
final TSUIDQuery tsuid_query =
new TSUIDQuery(tsdb, sub_query.getMetric(),
sub_query.getTags() != null ?
sub_query.getTags() : Collections.EMPTY_MAP);
if (data_query.getBackScan() > 0) {
deferreds.add(tsuid_query.getLastPoint(data_query.getResolveNames(),
...
} catch (Exception e) {
Throwable ex = e;
while (ex.getClass().equals(DeferredGroupException.class)) {
if (ex.getCause() == null) {
LOG.warn("Unable to get to the root cause of the DGE");
break;
}
ex = ex.getCause();
}
if (ex instanceof RuntimeException) {
throw new BadRequestException(ex);
} else {
throw new RuntimeException("Shouldn't be here", e);
}src/meta/TSUIDQuery.java
public Deferred<IncomingDataPoint> getLastPoint(final boolean resolve_names,
final int back_scan) {
...
class TSUIDCB implements Callback<Deferred<IncomingDataPoint>, byte[]> {
...
final GetRequest get = new GetRequest(tsdb.dataTable(), key);
get.family(TSDB.FAMILY());
return tsdb.getClient().get(get).addCallbackDeferring(new LastPointCB());
...
if (tsuid == null) {
return tsuidFromMetric(tsdb, metric, tags)
.addCallbackDeferring(new TSUIDCB());
}
try {
// damn typed exceptions....
return new TSUIDCB().call(null);
} catch (Exception e) {
return Deferred.fromError(e);
}
}
...
private class LastPointCB implements Callback<Deferred<IncomingDataPoint>,
ArrayList<KeyValue>> {
...
final IncomingDataPoint dp =
new Internal.GetLastDataPointCB(tsdb).call(row);src/core/Internal.java
public static class GetLastDataPointCB implements Callback<IncomingDataPoint,
ArrayList<KeyValue>> {
...
final ArrayList<Cell> cells = extractDataPoints(row, row.size());
...
public static ArrayList<Cell> extractDataPoints(final ArrayList<KeyValue> row,
final int estimated_nvalues) {
final ArrayList<Cell> cells = new ArrayList<Cell>(estimated_nvalues);
for (final KeyValue kv : row) {
final byte[] qual = kv.qualifier();
final int len = qual.length;
final byte[] val = kv.value();
// when enable_appends set to true, should get qualifier and value from the HBase Column Value
if (kv.qualifier()[0] == AppendDataPoints.APPEND_COLUMN_PREFIX) {
int idx = 0;
int q_length = 0;
int v_length = 0;
while (idx < kv.value().length) {
q_length = Internal.getQualifierLength(kv.value(), idx);
v_length = Internal.getValueLengthFromQualifier(kv.value(), idx);
final byte[] q = new byte[q_length];
final byte[] v = new byte[v_length];
System.arraycopy(kv.value(),idx,q,0,q_length);
System.arraycopy(kv.value(),idx + q_length,v, 0, v_length);
idx += q_length + v_length;
final Cell cell = new Cell(q, v);
cells.add(cell);
}E.g. it could be hitting an ArrayIndexOutOfBoundsException here intermittently due to #2186, for certain timestamps/values.
src/tsd/BadRequestException.java
public BadRequestException(final Throwable cause) {
this(cause.getMessage(), cause);
}This variant of the BadRequestException can result in an object that has a null message.
manolama
Since trying out a relatively recent checkout of opentsdb master (post 2.4.0), the following error sometimes occurs:
I.e. it is hitting a
NullPointerExceptionwhile it is attempting to report another exception.The relevant code is in the following file:
src/tsd/HttpSerializer.javaexception.getMessage()returnsnull, and so aNullPointerExceptionis hit when attempting to call thelength()method on that.As such, it means the nature of the other exception that this function was trying to report gets hidden.
A simple fix would be to coerce the result of
exception.getMessage()to a string at the start of this function before callinglength(). But arguably, this initial capacity argument to StringBuilder is pretty arbitrary anyway, so it might was well just be a fixed constant rather than being a function ofexception.getMessage().length().This can occur when a plain
RuntimeExceptionoccurs somewhere else (without message) and then is converted directly into aBadRequestException(and so still has no message). There are several code paths where this can occur. (In this particular case, I strongly suspect that the other error triggering this particular case is #2186, e.g. possibly hitting anArrayIndexOutOfBoundsExceptioninsrc/core/Interval.javaextractDataPoints.)Note that this bug in the error handling appears to be really old (since 2013), but would only rarely get hit, since it would only get hit when a
RuntimeExceptionoccurs elsewhere, which would only be when there is a bug of that nature elsewhere in the opentsdb code.It would be good to fix this, so that other more significant exception error messages are not hidden from the logs.