Use JSON instead of "PHP-style nested object syntax" to send actual data types in the metadata

[sompylasar]

Currently, because bugsnag-js serializes metadata as "PHP-style nested objects" into the query string, the actual data types get lost.

bugsnag-js/src/bugsnag.js

Lines 753 to 756 in 9fc759f

	// Deeply serialize an object into a query string. We use the PHP-style
	// nested object syntax, `nested[keys]=val`, to support heirachical
	// objects. Similar to jQuery's `$.param` method.
	function serialize(obj, prefix, depth) {

Preserving the data types in the reports is important to debug certain type-related issues which are not uncommon in weakly typed JavaScript.

The data types could be preserved by using JSON.

The public Bugsnag API is advertised to take JSON payload: https://docs.bugsnag.com/api/error-reporting/#json-payload

[jmshal]

I would be interested to see the size difference of using JSON instead, as I know the keys kinda add some redundancy to the payload right now.

The main issue I see here is the fact that IE6 & 7 (and in some cases IE8) (caniuse.com) don't support JSON.stringify. But the notifier needs to support them.

I don't see an issue with conditionally stringifying the payload based on what's available in the browser - using the current method as a fallback if JSON.stringify isn't supported. It probably wouldn't be the best idea to include a JSON shim just for some of the legacy browsers.

[sompylasar]

On a quite nested payload (redux state of a large app), JSON is twice shorter:

serialize(params).length
22650

JSON.stringify(params).length
12378

Note that JSON.stringify throws on circular references in data structures, so you'd like to apply something like this: https://github.com/sindresorhus/serialize-error/blob/9ea5d5332d2cf6516de05166f31c883dd42bbc54/index.js before applying JSON.stringify (there isn't much additional code, especially if compressed).

Moreover, serialize-error is smarter in handing circular data structures than the current version of bugsnag-js because it does not limit the depth globally, rather it only prevents actual circular references.

[sompylasar]

I was wrong to include the raw, non-URL-encoded JSON size, but the URL-encoded size is still less:

serialize(params).length
22672

JSON.stringify(params).length
12398

encodeURIComponent(JSON.stringify(params)).length
17252

---

Btw, can the current Bugsnag API accept JSON payload in the query string, in a GET request?

[jmshal]

I was wrong to include the raw, non-URL-encoded JSON size, but the URL-encoded size it still less:

serialize(params).length
22672

JSON.stringify(params).length
12398

encodeURIComponent(JSON.stringify(params)).length
17252

That's not a terrible saving. Every little helps.

Btw, can the current Bugsnag API accept JSON payload in the query string, in a GET request?

It doesn't seem to be possible with the current "general" notifier endpoint, because GET requests can't have a body.

Though seeing as the payload is structured differently for the JS notifier anyway, if the current JS endpoint accepted both the JSON & "PHP-style" format, it would make it even easier - as far as the changes required in the notifier.

[sompylasar]

Though seeing as the payload is structured differently for the JS notifier anyway, if the current JS endpoint accepted both the JSON & "PHP-style" format, it would make it even easier - as far as the changes required in the notifier.

Are you in the Bugsnag team? There isn't any public documentation of that special "js" endpoint...

[sompylasar]

Oops looks like my example payload, being sent as query string (either serialize or JSON), may hit the URL length limit in some browsers: http://stackoverflow.com/questions/812925/what-is-the-maximum-possible-length-of-a-query-string (fortunately, it hasn't hit it with the Bugsnag API server yet).

[jmshal]

Are you in the Bugsnag team? There isn't any public documentation of that special "js" endpoint...

No, I'm not. And I'm not sure there is any documentation for it - but you can open up the network tab (in Chrome) and take a look at what gets sent. Or you could do a bit of code diving 😛

Oops looks like my example payload, being sent as query string (either serialize or JSON), may hit the URL length limit in some browsers: http://stackoverflow.com/questions/812925/what-is-the-maximum-possible-length-of-a-query-string (fortunately, it hasn't hit it with the Bugsnag API server yet).

Yeah that's the main issue, and why every little really does help.

[jmshal]

I took a look at an example event (from the app I work on), and these are my results.

JSON.stringify(params).length
4471

encodeURIComponent(JSON.stringify(params)).length
6915

serialize(params).length
7488

[sompylasar]

And I'm not sure there is any documentation for it - but you can open up the network tab (in Chrome) and take a look at what gets sent. Or you could do a bit of code diving 😛

Yes, I did that. I mean, to learn if there are other types of requests it can possibly accept.

[sompylasar]

I am thinking that an option to specify a custom transport to send requests would be useful here. This way an app could implement both sendBeacon (which is URL-limited as well) and POST JSON.

And I'm thinking unfortunately I'll have to abandon bugsnag-js for now and implement requests to the "general" Bugsnag API with JSON Payload. :(
I'd like to reuse what it adds on top, but these URL and serialization limitations are quite critical to me, as I'd like to be able to analyze the full app state in the error condition.

[foxyblocks]

@sompylasar I think the easiest way to accomplish what you are trying to do is with a beforeNotify callback. This is called immediately before the request is made. In it you could use your own serialization and request logic and still benefit from all the features of the library. Just be sure to return false from the callback or it will perform duplicate requests.

Bugsnag.beforeNotify = function(payload) {
  // custom serialization and request logic
 
 return false;
}

The only other thing you would be missing is this one check that we do to filter out indecipherable cross-domain and eval script errors, but you could also paste that into your beforeNotify callback.

We recommend however that you continue using the JS endpoint with the payload in the url query string (as this endpoint doesn't support posting JSON). The JS endpoint pulls out some metadata (ip address, user agent, referer) from the request itself because it is assumed to come from the user directly. If you wanted to use the main notifier endpoint you would have to collect this information manually and include it in the payload metadata in order to take advantage of it.

[sompylasar]

@wordofchristian Looks good, thank you! I missed this part If you want to halt the notification completely, return false from this function. in the docs.

I'm sure this

if (payload.lineNumber === 0 && (/Script error\.?/).test(payload.message)) {

is not the only error that needs to be filtered. I previously made a custom error reporter, similar to bugsnag but proprietary, for a world-wide product, and we filtered lots of false reports including this one (there was a hundred lines of code just for filtering false positives observed in different browsers and conditions).

We recommend however that you continue using the JS endpoint with the payload in the url query string (as this endpoint doesn't support posting JSON). The JS endpoint pulls out some metadata (ip address, user agent, referer) from the request itself because it is assumed to come from the user directly. If you wanted to use the main notifier endpoint you would have to collect this information manually and include it in the payload metadata in order to take advantage of it.

If this endpoint could support consuming JSON payload, this could help partially, but this cannot help overcome the URL length limits that may apply. I need to post a large chunk of data and I'd like it to be structured to be able to use Bugsnag custom filters. What I dream of is to be able to apply filtering and tagging in the reporting service (backend + web UI) itself, not in the app that sends the reports.

[kattrali]

There's a lot of good points and interesting discussion in here. It makes sense that we give more thought to supporting a more modern delivery mechanism in addition to the existing options, though the library's API for handling this would need to be refined somewhat. I posted some related ideas on @jacobmarshall's related issue here. The next steps are to explore and define this idea more concretely, hopefully to have an expressive yet performant (and backwards compatible) library.

[sompylasar]

@kattrali Thanks! By the way, if you're going to redefine the library's API, please consider unifying bugsnag-js and bugsnag-node APIs for the common functions.

[kattrali]

@sompylasar That’s a great point too. Thanks!

[bengourley]

Hey everybody on this thread, you can now try out the v4 beta version of this notifier which uses JSON as the serialized data format (at last 🎉)!

Intro/instructions are here: https://docs.google.com/document/d/1HBBLyVv9FHsNmbkRLx58WbtisMRVHwDoR3pnxac9JQQ/

If you try the beta, please let us know how you get on. Otherwise we anticipate doing a full major release at the start of December.

[bengourley]

🚢 v4 is now published. Please upgrade!

See UPGRADING.md and full docs for more info 😄