Codimd: Error when creating notes with free url

[sunbit]

Having allowfreeurl set to true, an error raises:

2017-04-11T10:51:43.884Z - error:  TypeError: Cannot read property 'length' of undefined
at Model.<anonymous> (/private/var/projects/hackmd/hackmd/lib/models/revision.js:242:35)

It seems that it's trying to create a revision, but as the note still doesn't have content, it fails when accessing content properties. I patched lies 240:241 with

lastContent: note.content ? note.content : '',
length: note.content ? note.content.length : 0,

It works for me, but i suspect it would be better that the note have a default empty content?

[jackycute]

Hi @sunbit
Which version are you using?
We do give the default value for note content here:
https://github.com/hackmdio/hackmd/blob/master/lib/models/note.js#L65
So this is a pretty strange one.

[sunbit]

I've just checked out from master today,

commit a39870f224320856d43eb3377343f1c4229f11dd
Author: Yukai Huang <[email protected]>
Date:   Tue Apr 11 16:39:47 2017 +0800

Enable source map in development build

This is the last entry in git log. Also i must say that although it fails with a 500 Internal Error wtf. and the error that i previously attached, the second time you try to fetch the same url, it works, so it seems that even failing with the revision thing, it creates the note.

[telegnom]

Hey!
Same problem here. When trying to create a pad with custom url I get an error 500 in the first attempt. After reloading the page, the pad with the desired url is created. From now on the created pad works fine.

VersionL 0.5.1

[SISheogorath]

@jackycute I think I found the problem ~(even when I do not understand why it works in the general creation ._.)~

We use a getter here: https://github.com/hackmdio/hackmd/blob/master/lib/models/note.js#L65

The problem is that there is no defaultValue defined for the note content model. As the sequelize source show here: https://github.com/sequelize/sequelize/blob/v3/lib/dialects/sqlite/query.js#L193

They use undefined if no other defaultValue is defined by the model or by the value type.

This way your function processData defaults to return the value itself (undefined).

Since the value is undefined it has no properties and .length fails: https://github.com/hackmdio/hackmd/blob/master/lib/models/revision.js#L240-L241

This actually fails ALL THE TIME. The only reason it only is visible in case of a freeURLNote is that by a regular creation it's not caught: https://github.com/hackmdio/hackmd/blob/ca959012047bef3c0b2dd6f2974a8373a6615125/lib/response.js#L116

But in case of a free URL note it is: https://github.com/hackmdio/hackmd/blob/ca959012047bef3c0b2dd6f2974a8373a6615125/lib/response.js#L145-L162

And that's it! Took me a while to find and verify it, but yes, that's the problem.

[jackycute]

Thanks @SISheogorath helping us out here.
But actually the newNote should also catch error.

This might can be solved by giving content defaultValue.

[SISheogorath]

I'm not sure, who catches it, because you instantly run a redirect the request. And the error itself is not logged, so there are two options:

1. There is a catch that doesn't log the error
2. It's doesn't produce the exception, which makes no sense to me, but who knows