Skip to content

Commit

Permalink
Merge pull request #1203 from chamil321/exit
Browse files Browse the repository at this point in the history
Exit the application when a panic occurs
  • Loading branch information
chamil321 authored Sep 8, 2022
2 parents a01986b + 7346641 commit 87d1867
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 39 deletions.
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 @@ -283,7 +283,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 @@ -298,7 +298,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 @@ -2339,6 +2340,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);
}
}

0 comments on commit 87d1867

Please sign in to comment.