VA-1963 wrapping date deserialization #148
Conversation
…he client can decide if it wants to crash or not
…at way there can be no misconfiguration
|
I will look at this later today |
| @@ -125,7 +125,7 @@ private String toPrettyFormat(String jsonString) { | |||
| JsonParser parser = new JsonParser(); | |||
| JsonObject json = parser.parse(jsonString).getAsJsonObject(); | |||
|
|
|||
| Gson gson = new GsonBuilder().setPrettyPrinting().create(); | |||
| Gson gson = VimeoNetworkUtil.getGsonBuilder().setPrettyPrinting().create(); | |||
| * using the {@link ISO8601Utils} class that catches errors | ||
| * and reports them to the client application, allowing | ||
| * them to decide if they should crash or not. If we just | ||
| * rely on the default adapter used by JSON, we are unable |
| * and reports them to the client application, allowing | ||
| * them to decide if they should crash or not. If we just | ||
| * rely on the default adapter used by JSON, we are unable | ||
| * to absorb date parsing errors or log them correctly. |
There was a problem hiding this comment.
Also it wasn't just logging them incorrectly.....it was serializing it incorrectly.
There was a problem hiding this comment.
yeah I'll update these docs to reflect the new findings
|
Gson bug filed here google/gson#935 |
| try { | ||
| return json == null ? null : ISO8601Utils.parse(json.getAsString(), new ParsePosition(0)); | ||
| } catch (ParseException e) { | ||
| ClientLogger.e("Incorrectly formatted date sent from server: " + json.getAsString(), e); |
There was a problem hiding this comment.
See above, but maybe be a little more careful with getAsString?
| @@ -125,7 +125,7 @@ private String toPrettyFormat(String jsonString) { | |||
| JsonParser parser = new JsonParser(); | |||
| JsonObject json = parser.parse(jsonString).getAsJsonObject(); | |||
|
|
|||
| Gson gson = new GsonBuilder().setPrettyPrinting().create(); | |||
| Gson gson = VimeoNetworkUtil.getGsonBuilder().setPrettyPrinting().create(); | |||
There was a problem hiding this comment.
Do you think it would help to actually log the exception in the log below? Does it have any useful info?
There was a problem hiding this comment.
Maybe just log it then, it couldn't hurt. It is just one parameter...
| return new JsonDeserializer<Date>() { | ||
| @Override | ||
| public Date deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) | ||
| throws JsonParseException { |
There was a problem hiding this comment.
It doesn't actually throw a JsonParseException, so maybe ditch the the "throws"? It'll still be a legit overrride.
Also, if you just look at the javadocs for getAsString there are other types of exceptions that can occur here due to getAsString. Maybe we should swallow more than just ParseException? But perhaps more importantly, unless I am missing something...
Here's the weird thing regarding getAsString, in the version of gson that I have, this method always throws an UnsupportedOperationException. I'm confused...
There was a problem hiding this comment.
Regarding unsupported operation exception, it's because JsonElement is an abstract class, so the implementation is hidden
Ticket
VA-1963
Ticket Summary
The date deserialization is crashing sometimes because somehow dates are being serialized incorrectly.
Implementation Summary
How to Test
make sure dates still show up correctly