search

ottypes / json0

Version 0 of the JSON OT type
447 stars 64 forks source link

Do a more complete object type check to catch case that elem is undefined #5

Closed ishbu closed 9 years ago

ishbu commented 9 years ago

We've been seeing a rare occurence issue where elem during json.checkObj in apply is called on an undefined elem.

Still not sure of the root cause of the issue. But I believe this is a better object check that results in the error being thrown properly. Otherwise you get a stack trace like: 

TypeError: Cannot read property 'constructor' of undefined
at Object.json.checkObj (/var/lever/hire2/node_modules/derby/node_modules/racer/node_modules/share/node_modules/ottypes/lib/json0.js:103:11)
at Object.json.apply (/var/lever/hire2/node_modules/derby/node_modules/racer/node_modules/share/node_modules/ottypes/lib/json0.js:204:12)
at Object.json.incrementalApply (/var/lever/hire2/node_modules/derby/node_modules/racer/node_modules/share/node_modules/ottypes/lib/json0.js:240:21)
at Doc._otApply (/var/lever/hire2/node_modules/derby/node_modules/racer/node_modules/share/lib/client/doc.js:667:12)
at Doc._onMessage (/var/lever/hire2/node_modules/derby/node_modules/racer/node_modules/share/lib/client/doc.js:379:12)
at Connection.handleMessage (/var/lever/hire2/node_modules/derby/node_modules/racer/node_modules/share/lib/client/connection.js:215:37)
at StreamSocket.socket.onmessage (/var/lever/hire2/node_modules/derby/node_modules/racer/node_modules/share/lib/client/connection.js:124:18)
at StreamSocket.Channel.socket.onmessage (/var/lever/hire2/node_modules/derby/node_modules/racer/lib/Channel.js:19:28)
at Duplex._write (/var/lever/hire2/node_modules/derby/node_modules/racer/lib/Model/connection.server.js:23:12)
ishbu commented 9 years ago

I did run the tests and they seemed to pass, so not sure whats going on with the Travis CI build.

ishbu commented 9 years ago

Updated the tests to add a timeout matching the slow time, because mocha now defaults to 2s resulting in a failed test. Since the randomizers can run at arbitrary length depending on how many tests you want, we need the timeout to be longer.

I compared the time to run 1000 tests with the previous and the difference in completion time is negligible.

josephg commented 9 years ago

Cool, looks good. Good to know about mocha - that might explain some other failing tests I'm seeing all of a sudden.

ishbu commented 9 years ago

Thanks! Do you think you publish this to npm as well?

josephg commented 9 years ago

Sure.

josephg commented 9 years ago 
sephsmac:json0 josephg$ npm publish && git push && git push --tags
npm http PUT https://registry.npmjs.org/ot-json0
npm http 201 https://registry.npmjs.org/ot-json0
+ ot-json0@1.0.1