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

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

Open
techjeffharris wants to merge 5 commits intojfromaniello:masterfrom
techjeffharris:export_error_constants
Open

(semver-minor) define/export error constants#105
techjeffharris wants to merge 5 commits intojfromaniello:masterfrom
techjeffharris:export_error_constants

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
lib/index.js Outdated
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