Query error propagation

[pshc]

This patch ensures that errors during query processing are emitted by the relevant Query object. Essential for error handling on the client side. Also it does a sync() so that the connection keeps going.

The problem with it is that errors will get emitted twice, once from Query and once from Client. Not sure what to do about that—maybe temporarily suspend the Client's error listener when a query is happening?

I also updated the relevant test, but I'm not 100% on it due to both the double-error business and the fact that the type coercion tests were failing either way. Not sure what's up with that.

Thanks for your trouble!

[brianc]

I agree with what you're saying here and definitely think there needs to be a better error handling story. I haven't really put a lot of effort into the correct way to handle errors yet.

One thing though with this patch...I'm not sure you need to send a sync message to the server unless you're in the middle of processing a prepared statement when the error is encountered. What about hitting an error during a simple query? I think it might need a few more tests to ensure sync is only sent when it's needed. I was thinking of possibly writing a unit test or two using a "mock" stream to fake out when the errors come back. What you think?

If you check out `test/unit/client/prepared-statement-tests.js' there is an example of mocking out the prepared statement communication in there. I was thinking of adding "error" instead of the correct "Complete" response sometimes & checking for correct handling. I thought that might be easier than formulating a query which would correctly make PostgreSQL server throw an error at each step of the process.

Also: figured out what was going on w/ the type coercion tests. It had to do w/ the way I was loading a BUNCH of queries into the client's query queue and not waiting long enough for them to complete. Didn't show up until I tested on a slower machine. I've set up a really resource constrained linode instance now w/ NVM and PostgreSQL 8.4. So I can test against multiple versions of postgres and node. So...no more huge test failures hopefully! Sorry 'bout that.

[pshc]

I bet you're right about the synching. Didn't look at the spec closely enough. Mock stream sounds good. I'm familiarizing myself with your testing methods so I'll hopefully be writing more elaborate tests soon.

And that's what I get for developing on a netbook, ha!

[brianc]

yah man that's awesome you caught those timing issues on the queries. I pushed out some more updates to the tests to fix the timing stuff. I got two way date coercion working, but I think it impacts performance somewhat cause you gotta do an "instanceof" to every single parameter in every query. :( Not sure of any way around it without forcing the coercion on the user.