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

Exit the application when a panic occurs #1203

Merged
merged 5 commits into from
Sep 8, 2022
Merged
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
14 changes: 7 additions & 7 deletions ballerina-tests/tests/interceptors_error_handling_tests.bal
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ import ballerina/test;

final http:Client noServiceRegisteredClientEP = check new("http://localhost:" + noServiceRegisteredTestPort.toString(), httpVersion = http:HTTP_1_1);

listener http:Listener noServiceRegisteredServerEP = new(noServiceRegisteredTestPort,
listener http:Listener noServiceRegisteredServerEP = new(noServiceRegisteredTestPort,
httpVersion = http:HTTP_1_1,
interceptors = [
new LastResponseInterceptor(), new DefaultResponseErrorInterceptor(), new DefaultRequestInterceptor(),
new LastResponseInterceptor(), new DefaultResponseErrorInterceptor(), new DefaultRequestInterceptor(),
new DefaultRequestErrorInterceptor(), new LastRequestInterceptor(), new DefaultResponseInterceptor()
]
);
Expand All @@ -40,10 +40,10 @@ function testNoServiceRegistered() returns error? {

final http:Client serviceErrorHandlingClientEP = check new("http://localhost:" + serviceErrorHandlingTestPort.toString(), httpVersion = http:HTTP_1_1);

listener http:Listener serviceErrorHandlingServerEP = new(serviceErrorHandlingTestPort,
listener http:Listener serviceErrorHandlingServerEP = new(serviceErrorHandlingTestPort,
httpVersion = http:HTTP_1_1,
interceptors = [
new LastResponseInterceptor(), new DefaultResponseErrorInterceptor(), new DefaultRequestInterceptor(),
new LastResponseInterceptor(), new DefaultResponseErrorInterceptor(), new DefaultRequestInterceptor(),
new DefaultRequestErrorInterceptor(), new LastRequestInterceptor(), new DefaultResponseInterceptor()
]
);
Expand Down Expand Up @@ -233,7 +233,7 @@ function testConsumesProducesError() returns error? {
assertHeaderValue(check res.getHeader("error-type"), "DispatchingError-Resource");
}

listener http:Listener authErrorHandlingServerEP = new(authErrorHandlingTestPort,
listener http:Listener authErrorHandlingServerEP = new(authErrorHandlingTestPort,
httpVersion = http:HTTP_1_1,
interceptors = [
new LastResponseInterceptor(), new DefaultResponseErrorInterceptor(), new DefaultRequestInterceptor(),
Expand Down Expand Up @@ -282,7 +282,7 @@ function testAuthnError() returns error? {
assertHeaderValue(check res.getHeader("last-interceptor"), "default-response-error-interceptor");
assertHeaderValue(check res.getHeader("default-response-error-interceptor"), "true");
assertHeaderValue(check res.getHeader("last-response-interceptor"), "true");
assertHeaderValue(check res.getHeader("error-type"), "ListenerAuthenticationError");
assertHeaderValue(check res.getHeader("error-type"), "ListenerAuthorizationError");

clientEP = check new("https://localhost:" + authErrorHandlingTestPort.toString(),
secureSocket = {
Expand All @@ -297,7 +297,7 @@ function testAuthnError() returns error? {
assertHeaderValue(check res.getHeader("last-interceptor"), "default-response-error-interceptor");
assertHeaderValue(check res.getHeader("default-response-error-interceptor"), "true");
assertHeaderValue(check res.getHeader("last-response-interceptor"), "true");
assertHeaderValue(check res.getHeader("error-type"), "ListenerAuthenticationError");
assertHeaderValue(check res.getHeader("error-type"), "ListenerAuthorizationError");
}

@test:Config{}
Expand Down
6 changes: 3 additions & 3 deletions ballerina/auth_desugar.bal
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,12 @@ public isolated function authenticateResource(Service serviceRef, string methodN
if header is string {
Unauthorized|Forbidden? result = tryAuthenticate(<ListenerAuthConfig[]>authConfig, header);
if result is Unauthorized {
panic error ListenerAuthnError("");
panic error InternalListenerAuthnError("");
} else if result is Forbidden {
panic error ListenerAuthzError("");
panic error InternalListenerAuthzError("");
}
} else {
panic error ListenerAuthnError("");
panic error InternalListenerAuthnError("");
}
}

Expand Down
6 changes: 6 additions & 0 deletions ballerina/http_errors.bal
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ public type ListenerAuthnError distinct ListenerAuthError;
# Defines the authorization error types that returned from listener.
public type ListenerAuthzError distinct ListenerAuthError;

# Defined for internal use when panicing from the auth_desugar
type InternalListenerAuthnError distinct ListenerAuthError;

# Defined for internal use when panicing from the auth_desugar
type InternalListenerAuthzError distinct ListenerAuthError;

# Defines the client error types that returned while sending outbound request.
public type OutboundRequestError distinct ClientError;

Expand Down
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- [Make http2 the default transport in http](https://github.com/ballerina-platform/ballerina-standard-library/issues/454)
- [Add support for client resource methods in HTTP client](https://github.com/ballerina-platform/ballerina-standard-library/issues/3102)
- [Add constraint validation to HTTP payload binding](https://github.com/ballerina-platform/ballerina-standard-library/issues/3108)
- [Kill the application when a resource function panics](https://github.com/ballerina-platform/ballerina-standard-library/issues/2714)

### Changed
- [Update default response status as HTTP 201 for POST resources](https://github.com/ballerina-platform/ballerina-standard-library/issues/2469)
Expand Down
7 changes: 7 additions & 0 deletions docs/spec/spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ The conforming implementation of the specification is released and included in t
* 8.2.2. [Error types](#822-error-types)
* 8.2.3. [Trace log](#823-trace-log)
* 8.2.4. [Access log](#824-access-log)
* 8.2.5. [Panic inside resource](#825-panic-inside-resource)
9. [Security](#9-security)
* 9.1. [Authentication and Authorization](#91-authentication-and-authorization)
* 9.1.1. [Declarative Approach](#911-declarative-approach)
Expand Down Expand Up @@ -2327,6 +2328,12 @@ console = true # Default is false
path = "testAccessLog.txt" # Optional
```

#### 8.2.5 Panic inside resource

Ballering consider panic as a catastrophic error and non recoverable. Hence immediate application termination is
performed to fail fast after responding to the request. This behaviour will be more useful in cloud environments as
well.

## 9. Security

### 9.1 Authentication and Authorization
Expand Down
8 changes: 8 additions & 0 deletions native/spotbugs-exclude.xml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,14 @@
<Class name="io.ballerina.stdlib.http.transport.contractimpl.common.certificatevalidation.cache.CacheManager$CacheManagingTask$LRUEntryCollector" />
<Bug pattern="UPM_UNCALLED_PRIVATE_METHOD" />
</Match>
<Match>
<Class name="io.ballerina.stdlib.http.api.HttpCallableUnitCallback$1" />
<Bug pattern="DM_EXIT" />
</Match>
<Match>
<Class name="io.ballerina.stdlib.http.api.HttpResponseInterceptorUnitCallback$1" />
<Bug pattern="DM_EXIT" />
</Match>
<Match>
<Class name="io.ballerina.stdlib.http.api.HttpDispatcher" />
</Match>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,16 @@ private void returnErrorResponse(BError error) {
invokeBalMethod(paramFeed, "returnErrorResponse");
}

private void invokeBalMethod(Object[] paramFeed, String methodName) {
public void invokeBalMethod(Object[] paramFeed, String methodName) {
Callback returnCallback = new Callback() {
@Override
public void notifySuccess(Object result) {
stopObserverContext();
printStacktraceIfError(result);
Object isPanic = requestMessage.getProperty(HttpConstants.INTERCEPTOR_SERVICE_PANIC_ERROR);
if (Objects.nonNull(isPanic) && (boolean) isPanic) {
System.exit(0);
}
}

@Override
Expand All @@ -149,10 +153,12 @@ private void stopObserverContext() {
public void notifyFailure(BError error) { // handles panic and check_panic
cleanupRequestMessage();
// This check is added to update the status code with respect to the auth errors.
if (error.getType().getName().equals(HttpErrorType.LISTENER_AUTHN_ERROR.getErrorName())) {
if (error.getType().getName().equals(HttpErrorType.INTERNAL_LISTENER_AUTHN_ERROR.getErrorName())) {
requestMessage.setHttpStatusCode(401);
} else if (error.getType().getName().equals(HttpErrorType.LISTENER_AUTHZ_ERROR.getErrorName())) {
} else if (error.getType().getName().equals(HttpErrorType.INTERNAL_LISTENER_AUTHZ_ERROR.getErrorName())) {
requestMessage.setHttpStatusCode(403);
} else {
requestMessage.setProperty(HttpConstants.INTERCEPTOR_SERVICE_PANIC_ERROR, true);
}
if (alreadyResponded(error)) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,7 @@ public class HttpConstants {
public static final String REQUEST_INTERCEPTOR_INDEX = "REQUEST_INTERCEPTOR_INDEX";
public static final String RESPONSE_INTERCEPTOR_INDEX = "RESPONSE_INTERCEPTOR_INDEX";
public static final String INTERCEPTOR_SERVICE_ERROR = "INTERCEPTOR_SERVICE_ERROR";
public static final String INTERCEPTOR_SERVICE_PANIC_ERROR = "INTERCEPTOR_SERVICE_PANIC_ERROR";
public static final String WAIT_FOR_FULL_REQUEST = "WAIT_FOR_FULL_REQUEST";
public static final String HTTP_NORMAL = "Normal";
public static final String REQUEST_INTERCEPTOR = "RequestInterceptor";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ public enum HttpErrorType {
REQ_DISPATCHING_ERROR("RequestDispatchingError"),
SERVICE_DISPATCHING_ERROR("ServiceDispatchingError"),
RESOURCE_DISPATCHING_ERROR("ResourceDispatchingError"),
LISTENER_AUTHZ_ERROR("ListenerAuthzError"),
LISTENER_AUTHN_ERROR("ListenerAuthnError");
INTERNAL_LISTENER_AUTHZ_ERROR("InternalListenerAuthzError"),
INTERNAL_LISTENER_AUTHN_ERROR("InternalListenerAuthnError");

private final String errorName;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,19 @@
package io.ballerina.stdlib.http.api;

import io.ballerina.runtime.api.Environment;
import io.ballerina.runtime.api.PredefinedTypes;
import io.ballerina.runtime.api.Runtime;
import io.ballerina.runtime.api.async.Callback;
import io.ballerina.runtime.api.types.ServiceType;
import io.ballerina.runtime.api.values.BArray;
import io.ballerina.runtime.api.values.BError;
import io.ballerina.runtime.api.values.BObject;
import io.ballerina.stdlib.http.api.nativeimpl.ModuleUtils;
import io.ballerina.stdlib.http.api.nativeimpl.connection.Respond;
import io.ballerina.stdlib.http.transport.message.HttpCarbonMessage;

/**
* {@code HttpResponseInterceptorUnitCallback} is the responsible for acting on notifications received from Ballerina
* side when a response interceptor service is invoked.
*/
public class HttpResponseInterceptorUnitCallback implements Callback {
public class HttpResponseInterceptorUnitCallback extends HttpCallableUnitCallback {
private static final String ILLEGAL_FUNCTION_INVOKED = "illegal return: response has already been sent";

private final HttpCarbonMessage requestMessage;
Expand All @@ -43,18 +40,17 @@ public class HttpResponseInterceptorUnitCallback implements Callback {
private final Environment environment;
private final BObject requestCtx;
private final DataContext dataContext;
private final Runtime runtime;


public HttpResponseInterceptorUnitCallback(HttpCarbonMessage requestMessage, BObject caller, BObject response,
Environment env, DataContext dataContext, Runtime runtime) {
super(requestMessage, runtime);
this.requestMessage = requestMessage;
this.requestCtx = (BObject) requestMessage.getProperty(HttpConstants.REQUEST_CONTEXT);
this.caller = caller;
this.response = response;
this.environment = env;
this.dataContext = dataContext;
this.runtime = runtime;
}

@Override
Expand All @@ -72,7 +68,7 @@ public void notifyFailure(BError error) { // handles panic and check_panic
invokeErrorInterceptors(error, false);
}

private void invokeErrorInterceptors(BError error, boolean printError) {
public void invokeErrorInterceptors(BError error, boolean printError) {
requestMessage.setProperty(HttpConstants.INTERCEPTOR_SERVICE_ERROR, error);
if (printError) {
error.printStackTrace();
Expand Down Expand Up @@ -175,21 +171,4 @@ public void returnErrorResponse(BError error) {

invokeBalMethod(paramFeed, "returnErrorResponse");
}

private void invokeBalMethod(Object[] paramFeed, String methodName) {
Callback returnCallback = new Callback() {
@Override
public void notifySuccess(Object result) {
printStacktraceIfError(result);
}

@Override
public void notifyFailure(BError result) {
sendFailureResponse(result);
}
};
runtime.invokeMethodAsyncSequentially(
caller, methodName, null, ModuleUtils.getNotifySuccessMetaData(),
returnCallback, null, PredefinedTypes.TYPE_NULL, paramFeed);
}
}