Skip to content
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

[2201.7.x] Handle bal future complete panic by logging the error #2223

Draft
wants to merge 2 commits into
base: 2201.7.x
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import io.ballerina.stdlib.http.transport.contract.HttpClientConnector;
import io.ballerina.stdlib.http.transport.message.HttpCarbonMessage;

import static io.ballerina.stdlib.http.api.HttpConstants.FUTURE_COMPLETE_ERR_MSG;

/**
* {@code DataContext} is the wrapper to hold {@code Context} and {@code Callback}.
*/
Expand Down Expand Up @@ -55,17 +57,17 @@
public void notifyInboundResponseStatus(BObject inboundResponse, BError httpConnectorError) {
//Make the request associate with this response consumable again so that it can be reused.
if (inboundResponse != null) {
getFuture().complete(inboundResponse);
completeFuture(inboundResponse, "notify inbound response status");
} else if (httpConnectorError != null) {
getFuture().complete(httpConnectorError);
completeFuture(httpConnectorError, "notify inbound response connector error");
} else {
BError err = HttpUtil.createHttpError("inbound response retrieval error", HttpErrorType.CLIENT_ERROR);
getFuture().complete(err);
completeFuture(err, "notify inbound response retrieval error");

Check warning on line 65 in native/src/main/java/io/ballerina/stdlib/http/api/DataContext.java

View check run for this annotation

Codecov / codecov/patch

native/src/main/java/io/ballerina/stdlib/http/api/DataContext.java#L65

Added line #L65 was not covered by tests
}
}

public void notifyOutboundResponseStatus(BError httpConnectorError) {
getFuture().complete(httpConnectorError);
completeFuture(httpConnectorError, "notify outbound response connector error");
}

public HttpCarbonMessage getOutboundRequest() {
Expand All @@ -84,7 +86,12 @@
return environment;
}

public Future getFuture() {
return balFuture;
public void completeFuture(Object result, String methodCall) {
try {
balFuture.complete(result);
} catch (BError err) {
System.err.printf(FUTURE_COMPLETE_ERR_MSG, methodCall, err.getMessage());
HttpUtil.printStacktraceIfError(result);

Check warning on line 94 in native/src/main/java/io/ballerina/stdlib/http/api/DataContext.java

View check run for this annotation

Codecov / codecov/patch

native/src/main/java/io/ballerina/stdlib/http/api/DataContext.java#L92-L94

Added lines #L92 - L94 were not covered by tests
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public void invokeBalMethod(Object[] paramFeed, String methodName) {
@Override
public void notifySuccess(Object result) {
stopObserverContext();
printStacktraceIfError(result);
HttpUtil.printStacktraceIfError(result);
}

@Override
Expand Down Expand Up @@ -188,17 +188,11 @@ private boolean alreadyResponded(Object result) {
HttpUtil.methodInvocationCheck(requestMessage, HttpConstants.INVALID_STATUS_CODE, ILLEGAL_FUNCTION_INVOKED);
} catch (BError e) {
if (result != null) { // handles nil return and end of resource exec
printStacktraceIfError(result);
HttpUtil.printStacktraceIfError(result);
err.println(HttpConstants.HTTP_RUNTIME_WARNING_PREFIX + e.getMessage());
}
return true;
}
return false;
}

private void printStacktraceIfError(Object result) {
if (result instanceof BError) {
((BError) result).printStackTrace();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,8 @@ public class HttpConstants {

public static final BString REQUEST_CTX_MEMBERS = StringUtils.fromString("members");

public static final String FUTURE_COMPLETE_ERR_MSG = "%s failed with bal future completion error: %s%n";

private HttpConstants() {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,6 @@ private boolean alreadyResponded() {
return false;
}

private void printStacktraceIfError(Object result) {
if (result instanceof BError) {
((BError) result).printStackTrace();
}
}

private void sendRequestToNextService() {
ballerinaHTTPConnectorListener.onMessage(requestMessage);
}
Expand Down Expand Up @@ -186,7 +180,7 @@ public void invokeBalMethod(Object[] paramFeed, String methodName) {
Callback returnCallback = new Callback() {
@Override
public void notifySuccess(Object result) {
printStacktraceIfError(result);
HttpUtil.printStacktraceIfError(result);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,6 @@ public void invokeErrorInterceptors(BError error, boolean printError) {
returnErrorResponse(error);
}

private void printStacktraceIfError(Object result) {
if (result instanceof BError) {
((BError) result).printStackTrace();
}
}

private void sendResponseToNextService() {
Respond.nativeRespondWithDataCtx(environment, caller, response, dataContext);
}
Expand Down Expand Up @@ -171,7 +165,7 @@ public void invokeBalMethod(Object[] paramFeed, String methodName) {
public void notifySuccess(Object result) {
stopObserverContext();
dataContext.notifyOutboundResponseStatus(null);
printStacktraceIfError(result);
HttpUtil.printStacktraceIfError(result);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1971,6 +1971,12 @@ public static boolean isHttpStatusCodeResponseTypeWithBody(Type type) {
return false;
}

public static void printStacktraceIfError(Object result) {
if (result instanceof BError) {
((BError) result).printStackTrace();
}
}

private HttpUtil() {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,16 @@

import io.ballerina.runtime.api.Environment;
import io.ballerina.runtime.api.Future;
import io.ballerina.runtime.api.values.BError;
import io.ballerina.runtime.api.values.BObject;
import io.ballerina.stdlib.http.api.HttpConstants;
import io.ballerina.stdlib.http.api.HttpUtil;
import io.ballerina.stdlib.http.transport.contract.HttpClientConnector;
import io.ballerina.stdlib.http.transport.contract.HttpClientConnectorListener;
import io.ballerina.stdlib.http.transport.message.ResponseHandle;

import static io.ballerina.stdlib.http.api.HttpConstants.FUTURE_COMPLETE_ERR_MSG;

/**
* {@code HasPromise} action can be used to check whether a push promise is available.
*/
Expand All @@ -51,7 +54,11 @@

@Override
public void onPushPromiseAvailability(boolean isPromiseAvailable) {
balFuture.complete(isPromiseAvailable);
try {
balFuture.complete(isPromiseAvailable);
} catch (BError err) {
System.err.printf(FUTURE_COMPLETE_ERR_MSG, "check promise availability", err.getMessage());

Check warning on line 60 in native/src/main/java/io/ballerina/stdlib/http/api/client/actions/HasPromise.java

View check run for this annotation

Codecov / codecov/patch

native/src/main/java/io/ballerina/stdlib/http/api/client/actions/HasPromise.java#L59-L60

Added lines #L59 - L60 were not covered by tests
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import static io.ballerina.stdlib.http.api.HttpConstants.CURRENT_TRANSACTION_CONTEXT_PROPERTY;
import static io.ballerina.stdlib.http.api.HttpConstants.EMPTY;
import static io.ballerina.stdlib.http.api.HttpConstants.EQUAL_SIGN;
import static io.ballerina.stdlib.http.api.HttpConstants.FUTURE_COMPLETE_ERR_MSG;
import static io.ballerina.stdlib.http.api.HttpConstants.MAIN_STRAND;
import static io.ballerina.stdlib.http.api.HttpConstants.ORIGIN_HOST;
import static io.ballerina.stdlib.http.api.HttpConstants.POOLED_BYTE_BUFFER_FACTORY;
Expand Down Expand Up @@ -226,15 +227,27 @@
env.getRuntime().invokeMethodAsync(client, methodName, null, null, new Callback() {
@Override
public void notifySuccess(Object result) {
balFuture.complete(result);
try {
balFuture.complete(result);
} catch (BError err) {
System.err.printf(FUTURE_COMPLETE_ERR_MSG, "invoke client method with success result",
err.getMessage());
HttpUtil.printStacktraceIfError(result);

Check warning on line 235 in native/src/main/java/io/ballerina/stdlib/http/api/client/actions/HttpClientAction.java

View check run for this annotation

Codecov / codecov/patch

native/src/main/java/io/ballerina/stdlib/http/api/client/actions/HttpClientAction.java#L232-L235

Added lines #L232 - L235 were not covered by tests
}
}

@Override
public void notifyFailure(BError bError) {
BError invocationError =
HttpUtil.createHttpError("client method invocation failed: " + bError.getErrorMessage(),
HttpErrorType.CLIENT_ERROR, bError);
balFuture.complete(invocationError);
try {
balFuture.complete(invocationError);
} catch (BError err) {
System.err.printf(FUTURE_COMPLETE_ERR_MSG, "invoke client method with failure result",
err.getMessage());
HttpUtil.printStacktraceIfError(invocationError);

Check warning on line 249 in native/src/main/java/io/ballerina/stdlib/http/api/client/actions/HttpClientAction.java

View check run for this annotation

Codecov / codecov/patch

native/src/main/java/io/ballerina/stdlib/http/api/client/actions/HttpClientAction.java#L246-L249

Added lines #L246 - L249 were not covered by tests
}
}
}, propertyMap, PredefinedTypes.TYPE_NULL, paramFeed);
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import java.util.Locale;
import java.util.Objects;

import static io.ballerina.stdlib.http.api.HttpConstants.FUTURE_COMPLETE_ERR_MSG;
import static io.ballerina.stdlib.mime.util.EntityBodyHandler.constructBlobDataSource;
import static io.ballerina.stdlib.mime.util.EntityBodyHandler.constructJsonDataSource;
import static io.ballerina.stdlib.mime.util.EntityBodyHandler.constructStringDataSource;
Expand Down Expand Up @@ -229,7 +230,14 @@
}

private static void setReturnValuesAndNotify(Future balFuture, Object result) {
balFuture.complete(result);
try {
balFuture.complete(result);
} catch (BError error) {

Check warning on line 235 in native/src/main/java/io/ballerina/stdlib/http/api/nativeimpl/ExternHttpDataSourceBuilder.java

View check run for this annotation

Codecov / codecov/patch

native/src/main/java/io/ballerina/stdlib/http/api/nativeimpl/ExternHttpDataSourceBuilder.java#L235

Added line #L235 was not covered by tests
String resultType = result instanceof BError ? "error" : "success";
System.err.printf(FUTURE_COMPLETE_ERR_MSG, "return data source builder " + resultType + " result",
error.getMessage());
HttpUtil.printStacktraceIfError(result);

Check warning on line 239 in native/src/main/java/io/ballerina/stdlib/http/api/nativeimpl/ExternHttpDataSourceBuilder.java

View check run for this annotation

Codecov / codecov/patch

native/src/main/java/io/ballerina/stdlib/http/api/nativeimpl/ExternHttpDataSourceBuilder.java#L237-L239

Added lines #L237 - L239 were not covered by tests
}
}

private static void updateDataSourceAndNotify(Future balFuture, BObject entityObj,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@
"as the response has been already used.", inboundRequestMsg.getSequenceId());
}
BError httpError = HttpUtil.createHttpError(errorMessage, HttpErrorType.GENERIC_LISTENER_ERROR);
dataContext.getFuture().complete(httpError);
dataContext.completeFuture(httpError, "notify outbound dirty response");
return null;
}
outboundResponseObj.addNativeData(HttpConstants.DIRTY_RESPONSE, true);
Expand All @@ -107,7 +107,7 @@
HttpUtil.checkFunctionValidity(inboundRequestMsg, outboundResponseMsg);
} catch (BError e) {
log.debug(e.getPrintableStackTrace(), e);
dataContext.getFuture().complete(e);
dataContext.completeFuture(e, "notify function availability failure");
return null;
}

Expand Down Expand Up @@ -151,14 +151,14 @@
}
} catch (BError e) {
log.debug(e.getPrintableStackTrace(), e);
dataContext.getFuture().complete(
HttpUtil.createHttpError(e.getMessage(), HttpErrorType.GENERIC_LISTENER_ERROR));
dataContext.completeFuture(HttpUtil.createHttpError(e.getMessage(), HttpErrorType.GENERIC_LISTENER_ERROR),

Check warning on line 154 in native/src/main/java/io/ballerina/stdlib/http/api/nativeimpl/connection/Respond.java

View check run for this annotation

Codecov / codecov/patch

native/src/main/java/io/ballerina/stdlib/http/api/nativeimpl/connection/Respond.java#L154

Added line #L154 was not covered by tests
"notify outbound response sent error");
} catch (Throwable e) {
//Exception is already notified by http transport.
String errorMessage = "Couldn't complete outbound response: " + e.getMessage();
log.debug(errorMessage, e);
dataContext.getFuture().complete(
HttpUtil.createHttpError(errorMessage, HttpErrorType.GENERIC_LISTENER_ERROR));
dataContext.completeFuture(HttpUtil.createHttpError(errorMessage, HttpErrorType.GENERIC_LISTENER_ERROR),

Check warning on line 160 in native/src/main/java/io/ballerina/stdlib/http/api/nativeimpl/connection/Respond.java

View check run for this annotation

Codecov / codecov/patch

native/src/main/java/io/ballerina/stdlib/http/api/nativeimpl/connection/Respond.java#L160

Added line #L160 was not covered by tests
"notify outbound response completion error");
}
return null;
}
Expand Down
Loading