Skip to content
This repository has been archived by the owner on Sep 28, 2021. It is now read-only.

(semver-minor) define/export error constants #105

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

techjeffharris
Copy link
Contributor

defining and exporting constants to define various error messages
makes them easier and more reliable to compare and test

defining and exporting constants to define various error messages
makes them easier and more reliable to compare and test
var ERROR_NO_COOKIE_PARSER = 'cookieParser is required use require(\'cookie-parser\'), connect.cookieParser or express.cookieParser';
var ERROR_NO_SESSION = 'No session found';
var ERROR_PASSPORT_NOT_INITIALIZED = 'Passport was not initialized';
var ERROR_SESSION_STORE = 'Error in session store:\n';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The \n should probably not be in the constant.. I'll move it to the message concatenaed to it on line 79.

@jfromaniello
Copy link
Owner

I'd rather use the constant names you used as error codes?
And create an Error type that can accept code

@techjeffharris
Copy link
Contributor Author

Something like

function PassportSocketIoError (code) {
  this.message = code
}

PassportSocketIoError.prototype = new Error();

var ERROR_NO_SESSION = 'No session found';

// during authorization
return auth.fail(data, new PassportSocketIoError(ERROR_NO_SESSION), false, accept);

...?

* Created PassportSocketIoError class that extends Error prototypally
* replace `new Error(/* ... */)` with `new PassportSocketIoError(/* ... */)`
* Add ERR_* messages to PassportSocketIoError.prorotype
* export PassportSocketIoError class
@techjeffharris
Copy link
Contributor Author

I'd rather use the constant names you used as error codes?

Is this more to your liking?

var ERROR_USER_NOT_AUTHORIZED       = 'ERROR_USER_NOT_AUTHORIZED';
var ERROR_USER_NOT_FOUND            = 'ERROR_USER_NOT_FOUND';

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants