impl: relax json syntax rules for deserialization #165
Conversation
For some clients, workspace polling fails due to the following error: ``` com.squareup.moshi.JsonEncodingException: Use JsonReader.setLenient(true) to accept malformed JSON at path $ ``` I haven’t been able to reproduce this issue, even using the same version deployed at the client (2.20.2). This change is an attempt to relax the JSON parsing rules by enabling lenient mode, in the hope that it will resolve the issue on the client side.
|
In theory it should decrease the risk but I can provide any guarantee until I am able to fully understand why the response is malformed/different. |
|
My initial impression is that this would only hide some other bug. Maybe a good first step would be to see if we can improve the error message? Like if we could log the request path and response. |
This is what I'm trying to figure out in between other priorities. Unfortunately by the time the exception is raised the response body has already been consumed by Moshi's converter, so you can't read it again. The interceptors are also not really a viable option IMHO, they are called before the converters which means:
Right now I'm playing with custom moshi converters and converter factories. From the docs it looks like it can be done, but I might have to manually write a lot of boilerplate hidden in moshi's factories. What are your thoughts on this @code-asher ? |
|
Would it be possible to use That is, assuming the interceptor works the way I hope it does 🤞 |
…I calls Wraps Moshi converter to log raw response body when JSON parsing fails. It helps debug malformed JSON during workspace polling by logging full response content, content type, and error details when a exception during marshalling occurs. A couple of approaches were tried, unfortunately by the time the exception is raised the response body has already been consumed by Moshi's converter, so you can't read it again. The interceptors are also not really a viable option, they are called before the converters which means: - we don't know if the response body is in the correct form or not. This is problematic because it means for every rest call we have to read the body twice (interceptor and by moshi converter) - we also have to save the intercepted body and store it until we have an exception from moshi, in which case it will have to be logged. This approach only logs on conversion failures, and the only performance impact on successful responses is the fact that we convert the response body byte stream to a string representation that can later be printed, and then again back to a byte stream by the moshi converter.
For some clients, workspace polling fails due to the following error:
I haven’t been able to reproduce this issue, even using the same version deployed at the client (2.20.2). This change is an attempt to relax the JSON parsing rules by enabling lenient mode, in the hope that it will resolve the issue on the client side. On top of it I've add error logging for malformed JSON responses in Coder REST API calls by wrapping Moshi converters in our own custom converter which logs raw response body when JSON parsing fails. It helps debugging
malformed JSON during workspace polling by logging full response content, content type, and error details when a exception during marshalling occurs.
I tried a couple of approaches, unfortunately by the time the exception is raised the response body has already been consumed by Moshi's converter, so you can't read it again. The interceptors are also not really a viable option, they are called before the converters which means:
This approach only logs on conversion failures, and the only performance impact on successful responses is the fact that we convert the response body byte stream to a string representation that can later be printed, and then again back to a byte stream by the moshi converter.