WebReflection / dblite

sqlite for node.js without gyp problems
MIT License
209 stars 34 forks source link

Errors on callbacks #17

Closed erf closed 10 years ago

erf commented 10 years ago

Is there a reason errors are not returned on callbacks, like normally in node.js libs? I think it is more difficult to do tests and return error messages when you have to set an event handler on the db object.

WebReflection commented 10 years ago

right now you need to add the error event to the db but it's planned to have an err flag in the next version.

To test, db.on('error', failTest); should do for now.

erf commented 10 years ago

Ah, ok thanks. Looking forward to next release=)

WebReflection commented 10 years ago

FYI, version 0.4.0 has the err variable. Callbacks there work either with a single argument, or double.

function (rows) {} will keep working as expected for now

function (err, rows) {} will receive the error buffer if returned, null otherwise.

bartve commented 10 years ago

Question: Doesn't the error buffer just contain a string which the sqlite3 executable outputs on error? Wouldn't it make more sense to pass a new Error object with the flushed buffer in the message to the callback? Then err actually contains an Error instance. Just a thought. Or am I missing something?

WebReflection commented 10 years ago

I kept that as an object so a classic test if(err) would never fail but what you say might make sense (I've never seen node people writing instance of Error though, neither go for err.message) … will think about that

bartve commented 10 years ago

You're right with the if(err) as that is what everybody (including myself) uses, but with an instance of Error you get the standard property err.message to find out what is wrong. In my own little framework I have several functions that can have err as the first param, but I have standardized them all to an Error object so that they can all be handled in an uniform way. When needed, one can always extend the native Error with extra properties while easily maintaining backwards compatibility.

WebReflection commented 10 years ago

makes sense, will pass an Error with the string on it in 4.0.1

WebReflection commented 10 years ago

in, with an extra minor fix on handled errors. Let me know if it's OK now, cheers

bartve commented 10 years ago

Haven't tested it thoroughly, but it looks fine :+1:, thanks! At least I was able to clean up my own code because I don't need to manually set the error event handlers in my insert/update/delete functions to the given callbacks any more :)

erf commented 10 years ago

nice=) thanks!