Use proper ServiceException when impossible to parse json input.

[RaHery]

• Strip out all information about internal implementation in the error message.
• for AOD-8242

---

This change is 

[llbrt]

Maybe you should remove extra spaces in empty lines (none present in original code).

Reviewed 4 of 4 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @clementdenis)

[alarribeau]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @clementdenis)

[clementdenis]

I don't see any support (or test) for the errors of type JsonProcessingException (item 2 in AOD-8242)
Do you plan to implement this separately, or was it forgotten?

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @RaHery)

---

endpoints-framework/src/main/java/com/google/api/server/spi/request/RestServletRequestParamReader.java, line 172 at r1 (raw file):

        | IOException e) {
      logger.atInfo().withCause(e).log("Unable to read request parameter(s)");
      throw new BadRequestException("Parse error", "parseError", "global");

I think it's still worth to propagate the error as the cause of the BadRequestException.
Also, there's no need for the "global" domain, it's the default value if not set.

---

endpoints-framework/src/main/java/com/google/api/server/spi/request/RestServletRequestParamReader.java, line 202 at r1 (raw file):

      );
    } else {
      return new BadRequestException("Parse error", reason, "global");

I would rather store the message in a variable and have a single BadRequestException created and returned at the end.
And I would also propagate the cause.

---

endpoints-framework/src/main/java/com/google/api/server/spi/request/RestServletRequestParamReader.java, line 222 at r1 (raw file):

            || float.class.equals(clazz)
            || double.class.equals(clazz);
  }

The type checks should use FieldType and its fromType() method instead to keep the logic centralized.

[RaHery]

JsonParseException and JsonEOFException are both IOException. Those exception, and any other exception related to the parsing that are IOException, are properly caught and message is just Parse error.
I added a test case to exhibit this feature.

Since this PR was already merged, i'll create another one with all of the modifications.

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @clementdenis)

---

endpoints-framework/src/main/java/com/google/api/server/spi/request/RestServletRequestParamReader.java, line 172 at r1 (raw file):

Previously, clementdenis (Clément Denis) wrote…

I think it's still worth to propagate the error as the cause of the BadRequestException.
Also, there's no need for the "global" domain, it's the default value if not set.

Done.

---

endpoints-framework/src/main/java/com/google/api/server/spi/request/RestServletRequestParamReader.java, line 202 at r1 (raw file):

Previously, clementdenis (Clément Denis) wrote…

I would rather store the message in a variable and have a single BadRequestException created and returned at the end.
And I would also propagate the cause.

Done.

---

endpoints-framework/src/main/java/com/google/api/server/spi/request/RestServletRequestParamReader.java, line 222 at r1 (raw file):

Previously, clementdenis (Clément Denis) wrote…

The type checks should use FieldType and its fromType() method instead to keep the logic centralized.

The logic are not the same.
There are similarities in the implementation, but the output is not the same, in value and semantically. The only thing in common is the type of input (Class) and the output (String). I think it's not worth it to factor those two pieces of code.