Fix reading malformed json record

[alesblaznik]

Seems like eventRecord.dataContentType === 1 is not reliable to check whether data is JSON valid.

(I have some events in EventStore written by API calls which have empty data, but dataContentType=1)

[x-cubed]

If we're just trying to deal with the Event Store returning an empty string, why are we falling back to binary decoding? If the data content type is JSON we should only support reading it as JSON.

I'm happy to have a try/catch around JSON.parse, but I think the fallback behaviour should be removed, and we should just return null if we can't parse it as JSON.

[alesblaznik]

I thought EventStore don't check if JSON is actually valid but that it just relies on the HTTP header you pass - if you're using HTTP API. But yes, it does validate.

The only case is then following: curl -XPOST http://127.0.0.1:2113/streams/my-stream -H"content-type:application/vnd.eventstore.events+json" -d "[{\"eventId\": \"6b73e70e-f5c9-41ed-aac4-a35d9f1ab282\", \"eventType\": \"NothingChanged\", \"data\": \"\"}]"

This is still flagged as JSON, but JSON.parse("") fails. Null might be returned in those cases, but why not just return raw data - might be other cases we don't know yet?

[x-cubed]

Ok, so you're saying that your application is trying to save an empty string as valid JSON? That doesn't sound like valid JSON to me, so the parse() call failing is to be expected.

This sounds like an application issue, not a client library issue.

Why isn't your application storing an empty object "{}" or "null"?

[x-cubed]

Also, FYI: https://stackoverflow.com/questions/30621802/why-does-json-parse-fail-with-the-empty-string

[alesblaznik]

Yep I totally agree about that. Bugs like this unfortunately happen and we need to get around.

Just trying to explain that it can happen to store "malformed" json to eventstore (see curl example above) and is still marked as json when reading from eventstore which causes parse error. I agree that it is a application error, but since it is already written we need a way to read it.

Would it make sense that instead of the try-catch, which really might get unexpected results, we put additional flag to connection.js (through options) which would allow to skip json parse, eg.

} else if (eventRecord.dataContentType === 1 && this.jsonDecode) {

having jsonDecode=true by default?

[x-cubed]

I don't think we need any extra options here, we just shouldn't propagate the exception if we can't parse the JSON. If we receive an event with the JSON data content type we should attempt to parse it as JSON and if we can't, just return a null value for the data property.

There's no useful information that gets lost here, it's just that an empty string isn't valid JSON.

[alesblaznik]

Agreed. I thought that EventStore does not do any JSON validation before storing events. I've tried couple of other examples and I got 400 responses (which I would expect for my example above).

I'll prepare a fix for that if we agree to return null value.

[x-cubed]

Cool, sounds good. Thanks.

[x-cubed]

What's the convertUuidToWtf function all about? It seems to be assuming that the source GUID is in a particular format, rather than actually checking anything. How does it handle GUIDs with the curly braces around them?

[alesblaznik]

Sorry, I forgot that I active have pull-request (this is mine dev branch I am trying things out).

What I noticed is that events written with this library don't store eventId properly - so when I was reading events, eventIds that I generated didn't match so I did some investigation. I found this thread on google groups: https://groups.google.com/forum/#!searchin/event-store/wtf$20uuid%7Csort:relevance/event-store/QadTjaxinw8/OlO79TU5CwAJ

--> as it seems all eventIds need to be converted to "wtf uuid" before written to eventstore. This little thing I did with the last commit worked for me.

You can try running your /example.js and you'll see that GUIDs generated don't match with eventId actually stored in eventstore.

[x-cubed]

No recent updates, closing this request.