Changed all errors to be instances of Error instead of plain strings #73

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

sebastianseilund commented Nov 28, 2012

Hi

This makes debugging a lot easier, and adds to the module's robustness.

The error for "Item is not stored" has a special use case, as it's something you expect could happen. The error object for this error message has a notStore=true property. It's useful like this:

memcached.add('only-set-me-once', Date.now(), 0, function(err) {
    if (err && !err.notStored)) {
        //Only throw the error, if it wasn't the notStored "error", which we expect to happen sometimes.
        throw err;
    }
    //Callback or whatever you wanna do
});

Unit tests pass after the changes.

Sebastian

Owner

3rd-Eden commented Nov 29, 2012

@sebastianseilund The pull request cannot be merged automatically, could you rebase with the current master?

Also some side information why I left "strings" as error messages... It's basically legacy code from around node 0.1.100, changing it would break backwards compatibility for some users. So I was planning to do this once I finished my new streaming parser (but it takes longer then expected :p)

Anyways, thanks for the pull request, love to merge this in and do a major version bump due to breaking API changes.

Grzesiek and others added some commits Nov 13, 2012

Fixed issue with values starting with keywords such as OK, VALUE etc. (
…#59) by escaping line breaks in values before set commands, and unescaping after get command.

- Added unit tests for OK and for VALUE
- Added unit test for checking correctly escaped line breaks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment