Merge pull request #4196 from vespa-engine/hmusum/return-internal-server-error-when-nullpointerexception

Return status code 500 and print stack trace if NullPointerException …

2 files changed, 23 insertions, 2 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java
index d869b5a2901..8f2cc04fad7 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java
@@ -15,8 +15,10 @@ import com.yahoo.config.provision.Rotation;
 import com.yahoo.config.provision.Version;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.lang.SettableOptional;
+import com.yahoo.log.LogLevel;
 import com.yahoo.vespa.config.server.ConfigServerSpec;
 import com.yahoo.vespa.config.server.deploy.ModelContextImpl;
+import com.yahoo.vespa.config.server.http.InternalServerException;
 import com.yahoo.vespa.config.server.http.UnknownVespaVersionException;
 import com.yahoo.vespa.config.server.provision.StaticProvisioner;
 
@@ -97,7 +99,12 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> {
             catch (RuntimeException e) {
                 boolean isOldestMajor = i == majorVersions.size() - 1;
                 if (isOldestMajor) {
-                    throw new IllegalArgumentException(applicationId + ": Error loading model", e);
+                    if (e instanceof NullPointerException) {
+                        log.log(LogLevel.WARNING, "Unexpected error when building model ", e);
+                        throw new InternalServerException(applicationId + ": Error loading model", e);
+                    } else {
+                        throw new IllegalArgumentException(applicationId + ": Error loading model", e);
+                    }
                 } else {
                     log.log(Level.INFO, applicationId + ": Skipping major version " + majorVersions.get(i), e);
                 }
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java
index d0e60144125..7900a67bddd 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java
@@ -32,6 +32,7 @@ import com.yahoo.vespa.config.server.http.*;
 import com.yahoo.vespa.config.server.session.*;
 import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.curator.mock.MockCurator;
+import org.apache.commons.lang.NullArgumentException;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -315,7 +316,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest {
 
     @Test
     public void test_out_of_capacity_response() throws InterruptedException, IOException {
-        String message = "No nodes available";
+        String message = "Internal error";
         SessionThrowingException session = new SessionThrowingException(new OutOfCapacityException(message));
         localRepo.addSession(session);
         HttpResponse response = createHandler()
@@ -327,6 +328,19 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest {
     }
 
     @Test
+    public void test_that_nullpointerexception_gives_internal_server_error() throws InterruptedException, IOException {
+        String message = "No nodes available";
+        SessionThrowingException session = new SessionThrowingException(new NullPointerException(message));
+        localRepo.addSession(session);
+        HttpResponse response = createHandler()
+                .handle(SessionHandlerTest.createTestRequest(pathPrefix, HttpRequest.Method.PUT, Cmd.PREPARED, 1L));
+        assertEquals(500, response.getStatus());
+        Slime data = getData(response);
+        assertThat(data.get().field("error-code").asString(), is(HttpErrorResponse.errorCodes.INTERNAL_SERVER_ERROR.name()));
+        assertThat(data.get().field("message").asString(), is(message));
+    }
+
+    @Test
     public void test_application_lock_failure() throws InterruptedException, IOException {
         String message = "Timed out after waiting PT1M to acquire lock '/provision/v1/locks/foo/bar/default'";
         SessionThrowingException session =