685945 - Log a warning if JSON parsing of bodies fails

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Richard Newman [:rnewman]]

Right now we just silently fail to set .payload, which leads to obscure errors later.

  deserialize: function deserialize(json) {
    this.data = json.constructor.toString() == String ? JSON.parse(json) : json;

    try {
      // The payload is likely to be JSON, but if not, keep it as a string
      this.payload = JSON.parse(this.payload);
    } catch(ex) {}
  },

It would be really swell to, y'know, log something if we get a body we can't parse -- maybe only if the response headers indicate a Content-Type of JSON, if we want to be neat and tidy about it.

Comment 1

[Richard Newman [:rnewman]]

Attachment: About time. v1

Example output:

1317890048133	Sync.Resource	WARN	Got exception "JSON.parse: unexpected character JS Stack trace: ()@resource.js:392 < ()@XPCOMUtils.jsm:210 < run_test()@test_resource.js:192 < _execute_test()@head.js:326 < @-e:1" parsing response body.
1317890048133	Sync.Resource	DEBUG	Parse fail: Response body starts: "This path exists".

Comment 2

[:Atoll]

Comment on attachment 565164 [details] [diff] [review]
About time. v1

>+        this._log.debug("Parse fail: Response body starts: \"" + (ret + "").slice(0, 100) + "\".");

i asked about encoding the slice so it's log-safe and :rnewman suggests that wrapping slice(0, 100) in "JSON stringification, whilst ugly, would be non-printable-safe".

Comment 3

[Richard Newman [:rnewman]]

Attachment: JSONifying, some cleanup. v2

Comment 4

[Philipp von Weitershausen [:philikon]]

Comment on attachment 565167 [details] [diff] [review]
JSONifying, some cleanup. v2

r=whisky

Comment 5

[Richard Newman [:rnewman]]

Pushed:

https://hg.mozilla.org/services/services-central/rev/ffc15889906e

Comment 6

[Richard Newman [:rnewman]]

Missed a qref after fixing tests to handle JSONifying. Fixxored.

https://hg.mozilla.org/services/services-central/rev/6270ac0e7f96

Comment 7

[Philipp von Weitershausen [:philikon]]

https://hg.mozilla.org/mozilla-central/rev/ffc15889906e
https://hg.mozilla.org/mozilla-central/rev/6270ac0e7f96