senecajs / seneca-transport

Seneca micro-services message transport over TCP and HTTP.
MIT License
63 stars 45 forks source link

use Jsonic over JSON.parse #65

Closed GlenTiki closed 8 years ago

GlenTiki commented 8 years ago

Turns out in #63 I had "bad" json being passed sent to my server. Instead of trying to parse the input with JSON.parse, maybe we should use jsonic to parse it?

mcollina commented 8 years ago

JSonic is slow, why would you like Seneca transport to be slower than it is?

I'm -1 on this change.

mcollina commented 8 years ago

93 should be fixed with better error handling, not lax parsing.

The transport is 'built for' seneca clients, humans do it only for testing.

GlenTiki commented 8 years ago

this fixed the error handling: https://github.com/senecajs/seneca-transport/pull/64

geek commented 8 years ago

@mcollina I agree, and was torn. I think instead we need to speed up jsonic for the common case, when everything is well formed.

mcollina commented 8 years ago

Having a lax parser for is bad, nobody would want that in production (I won't).

Speeding up jsonic to the level of JSON.parse will probably be quite hard (if not impossible), that piece of code is really highly optimized.

What it's the best approach is to return the user an output telling him it's wrong json, and to fix its syntax.

Also, this puts emphasis on write "broken" code, which we should not be supporting in the first place.

mcdonnelldean commented 8 years ago

I agree with what @mcollina says. While I agree with JSonic as a way for use to provide brevity shortcuts for actions (although it does need some perf work). I think it should be avoided outside of that use case.

geek commented 8 years ago

See response in #64 ... I reverted this change @thekemkid. Nothing was published.