Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeError: Cannot read property 'service' of undefined (continued) #108

Closed
thosakwe opened this Issue Mar 18, 2016 · 9 comments

Comments

Projects
None yet
3 participants
@thosakwe
Copy link

thosakwe commented Mar 18, 2016

Having a similar problem to #56.

Here's my app setup right now: Gist

I'm wondering what I messed up with.

Thanks in advance for your help, I appreciate that the Feathers team is responsive. It encourages me to keep using this framework.

@daffl

This comment has been minimized.

Copy link
Member

daffl commented Mar 18, 2016

The Gist looks right so far but how are you using the exported app? The error most likely happens it app.setup is not called. It gets called automatically when you use app.listen but if you e.g. use it with a virtual host or HTTP server you need to call app.setup with the server instance.

@thosakwe

This comment has been minimized.

Copy link
Author

thosakwe commented Mar 18, 2016

@daffl Here's the server file: https://gist.github.com/thosakwe/280e808bcc2175b273f1#file-server-js

I changed it now to use app.listen(port), but now I have a somewhat mysterious new error when trying to authenticate via auth/local. When my credentials are correct, I get this error: "data and hash arguments required."

data and hash arguments required

@daffl

This comment has been minimized.

Copy link
Member

daffl commented Mar 18, 2016

How doe the request and AppHooks.Users.create() look like? It looks like AuthenticationHooks.hashPassword() is trying to hash a password that is undefined.

@thosakwe

This comment has been minimized.

Copy link
Author

thosakwe commented Mar 18, 2016

Validator is feathers-validator. Long story short, it checks the hook's data for input errors.

function create() {
    return (hook) => {
        let errors = new Validator(hook.data, {
            username: "required|min:5|max:255",
            password: "required|min:6",
            email: "required|min:5|max:255"
        }).errors();

        if (errors.length) throw new Error(errors[0]);
    }
}
@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Mar 18, 2016

@thosakwe double check that your password field is there. That's a typical cause for that brypt error. I'm in the process of adding better error messaging for that.

As an aside in your hook you might want to throw new errors.BadRequest({ errors: errors }); so that you get the proper error code.

You also might want to check out @kulakowka's https://github.com/kulakowka/feathers-validate-hook. Maybe you guys can collaborate 😄

@thosakwe

This comment has been minimized.

Copy link
Author

thosakwe commented Mar 18, 2016

@ekryski Cool, I might contact him sooner or later.

Anyways, that just leaves me with one more question. I hooked hashPassword onto the update and patch methods as well.

Since the create method is already hooked to deny requests if the password isn't there, I'm wondering if the bcrypt error is because I didn't check for a password on update or patch.

Should I just remove hashPassword from update and patch? That might be the problem.

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Mar 18, 2016

@thosakwe oh ya that's probably the issue. hashPassword should only be called if a password is present, usually just on the create method. We are working on a password service that will manage changing passwords if that is what you are after.

@thosakwe

This comment has been minimized.

Copy link
Author

thosakwe commented Mar 19, 2016

I was doing all the above steps, and was getting nowhere. I looked over my setup again, and I realized that I overlooked the order of the hooks on my update and patch methods. I had verifyToken() after populateUser().

Thanks again for the help.

@thosakwe thosakwe closed this Mar 19, 2016

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Mar 19, 2016

@thosakwe glad you got it sorted. I have better error messages that will help hopefully make this more clear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.