Make the malformed response error message debuggable #4177
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4177 +/- ##
=======================================
Coverage 92.21% 92.21%
=======================================
Files 116 116
Lines 8089 8089
=======================================
Hits 7459 7459
Misses 630 630
Continue to review full report at Codecov.
|
| @@ -191,7 +191,11 @@ function wrapToHTTPRequest(hook, key) { | |||
| try { | |||
| body = JSON.parse(body); | |||
| } catch (e) { | |||
| err = { error: "Malformed response", code: -1 }; | |||
| err = { | |||
There was a problem hiding this comment.
does the e: SyntaxError have any useful info?
think this is worth unit testing??
There was a problem hiding this comment.
The case i'm trying to cover here isn't actually bad JSON it's when you don't have JSON at all and the text reads "Socket closed because not https" or something like that. We could also provide the e.toString() or e.message though for when there is a problem with the json formatting (but I think that is much less common since these are usually machine serialized results.
There was a problem hiding this comment.
This is definitely useful but feels sloppy, any other ideas on how to make this better @acinader?
There was a problem hiding this comment.
I wouldn't over think it. adds good feedback on an error. I approved it cause I think it's mergeable.
It would be nice if this error message contained some or all of the malformed response to aid in debugging the problem. What do you think?