From 178c7fc66dd2b50b6fa3349da2b7635a8d0b6a30 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Sat, 18 Nov 2017 10:47:06 +0100 Subject: Return status code 500 and print stack trace if NullPointerException is thrown When building a model, return 500 (as opposed to 400 now) and print stack trace when a NullPointerException is thrown --- .../vespa/config/server/modelfactory/ModelsBuilder.java | 8 +++++++- .../config/server/http/v2/SessionPrepareHandlerTest.java | 16 +++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) (limited to 'configserver') 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..801ddfbf585 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 @@ -17,6 +17,7 @@ import com.yahoo.config.provision.Zone; import com.yahoo.lang.SettableOptional; 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 +98,12 @@ public abstract class ModelsBuilder { catch (RuntimeException e) { boolean isOldestMajor = i == majorVersions.size() - 1; if (isOldestMajor) { - throw new IllegalArgumentException(applicationId + ": Error loading model", e); + if (e instanceof NullPointerException) { + e.printStackTrace(); + 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() @@ -326,6 +327,19 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { assertThat(data.get().field("message").asString(), is(message)); } + @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'"; -- cgit v1.2.3 From b273d9ae93bd6b16ebe7c27ea4b3334984a69033 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Mon, 20 Nov 2017 21:59:53 +0100 Subject: Log properly --- .../java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'configserver') 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 801ddfbf585..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,6 +15,7 @@ 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; @@ -99,7 +100,7 @@ public abstract class ModelsBuilder { boolean isOldestMajor = i == majorVersions.size() - 1; if (isOldestMajor) { if (e instanceof NullPointerException) { - e.printStackTrace(); + 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); -- cgit v1.2.3