zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-05-12 22:38:16 +02:00
Code Issues Projects Releases Packages Wiki Activity

Make ListObjectsAction return 200 when sending JSON error

Browse source 
This fixes a bug in the interaction between ListObjectsAction and ListObjectsCommand/AppEngineConnection.  ListObjectsAction was returning HTTP status code 400 when it caught an IAE, but also attempting to return a JSON response payload of {"status": "error", "error": "<exception message>"}.  However, AppEngineConnection treats any HTTP error response as more like a crash on the server side - it attempts to scrape the error message out of the autogenerated HTML that AppEngine produces for uncaught exceptions, and throws an exception, killing ListObjectsCommand before it can extract the JSON which contains the nicer error (that stating the missing field, etc versus just "400 Bad Request").

The fix is just to have ListObjectsAction return a 200 and the error message so that ListObjectsCommand can correctly handle it.

I also de-scoped the catch to only catching IAE, since catching Exception was overbroad, and the only "expected" exception to be thrown is an IAE from the checkArgument() that tests if the requested fields all exist.  Any other kinds of exceptions should actually just bubble up and kill the action, and get the regular AppEngineConnection error treatment.

I also added "billingId" as an alias for "billingIdentifier", parallel to clientId/clientIdentifier, since that's why I came across this issue in the first place.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=148248834
This commit is contained in:
nickfelt 2017-02-22 11:48:38 -08:00 • committed by Ben McIlwain
parent 9ea38e6348
commit 16832323d0
9 changed files with 20 additions and 34 deletions
Download patch file Download diff file Expand all files Collapse all files

18
java/google/registry/tools/server/ListObjectsAction.java
View file

@ -14,9 +14,8 @@
package google.registry.tools.server;
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkArgument;
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
import com.google.common.base.Function;
import com.google.common.base.Functions;
@ -36,6 +35,7 @@ import com.google.common.collect.Ordering;
import google.registry.model.ImmutableObject;
import google.registry.request.JsonResponse;
import google.registry.request.Parameter;
import google.registry.util.FormattingLogger;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
@ -53,6 +53,8 @@ import javax.inject.Inject;
*/
public abstract class ListObjectsAction<T extends ImmutableObject> implements Runnable {
private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass();
public static final String FIELDS_PARAM = "fields";
public static final String PRINT_HEADER_ROW_PARAM = "printHeaderRow";
public static final String FULL_FIELD_NAMES_PARAM = "fullFieldNames";
@ -117,13 +119,11 @@ public abstract class ListObjectsAction<T extends ImmutableObject> implements Ru
response.setPayload(ImmutableMap.of(
"lines", lines,
"status", "success"));
} catch (Exception e) {
String message = e.getMessage();
if (message == null) {
message = e.getClass().getName();
}
response.setStatus(
e instanceof IllegalArgumentException ? SC_BAD_REQUEST : SC_INTERNAL_SERVER_ERROR);
} catch (IllegalArgumentException e) {
String message = firstNonNull(e.getMessage(), e.getClass().getName());
logger.warning(e, message);
// Don't return a non-200 response, since that will cause RegistryTool to barf instead of
// letting ListObjectsCommand parse the JSON response and return a clean error.
response.setPayload(ImmutableMap.of(
"error", message,
"status", "error"));