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

User data not in returned user object #45

Closed
awalias opened this issue Jan 18, 2021 · 13 comments
Closed

User data not in returned user object #45

awalias opened this issue Jan 18, 2021 · 13 comments
Labels
documentation Improvements or additions to documentation help wanted Extra attention is needed released

Comments

@awalias
Copy link
Member

awalias commented Jan 18, 2021

May be just a documentation issue:

I noticed that the docs for auth.signUp() destructures user and error out of the return from signUp(), but the user data is in data. I thought I'd let you know if this is an error. It threw me for a loop until I realized to look at the whole object.

https://supabase.io/docs/client/auth-signup

@awalias awalias added documentation Improvements or additions to documentation help wanted Extra attention is needed labels Jan 18, 2021
@kiwicopple
Copy link
Member

@awalias where did you get this one from?

We are returning user here: https://github.com/supabase/gotrue-js/blob/8b61cc39df22cf9e1d546df7e1d06e9b27c51556/src/GoTrueClient.ts#L109

We also return data, but that's for legacy reasons (so we shouldn't document it) - eventually we will remove it

@mrkldshv
Copy link

mrkldshv commented Jan 24, 2021

@kiwicopple I got same behaviour when I was playing around with this example. I was trying to destructure user from signUp call and was getting null. Eventually I tried to use data property instead of user and that worked for me.

@laurentS
Copy link

laurentS commented Feb 7, 2021

@kiwicopple I just ran into this with gotrue-js 1.10.5, running against your servers.
After looking at the code a bit, I think what is happening is that the server returns the user as data (ie: data === user, and data.user === undefined), so that the line you quote above results in { data, user: null, error: null}.

As an example, I just ran this:

    const res = await supabase.auth.signUp({ email, password });
    console.log("res", res);

and the output looks like (in devtools):
image

Basically I think there's a mismatch in gotrue-js between the type declared for the return value of signUpWithEmail (which is supposed to return a Session) and what the server sends (which looks like a User).

@kiwicopple
Copy link
Member

OK it looks like GoTrue returns a full session if there is no email confirmation

image

And only the user object if you have email confirmations on:

image

I added a PR here, will merge tomorrow if there are no other comments

@mrkldshv
Copy link

mrkldshv commented Feb 7, 2021

Hey @kiwicopple, I have just tried to use signUp method with email confirmation turned on, however I wasn't able to get user object on it. That's the response:
Screen Shot 2021-02-07 at 18 53 57

@laurentS
Copy link

laurentS commented Feb 7, 2021

@kiwicopple I wonder if changing the public interface of your API depending on a setting is the clearest thing to do, from the user perspective. Is there any reason why you're not always returning a Session with session.user set no matter what, but the rest of the session fields being null or empty strings (I'm no expert in auth, just curious as it's not how I would have thought of fixing the issue)?

@kiwicopple
Copy link
Member

I wonder if changing the public interface of your API depending on a setting is the clearest thing to do

I agree, but this is from Netlify's GoTrue server. If this was our own server I would have preferred consistency. We could change this in our own fork, but I don't think Netlify would ever merge it upstream.

However I can make it so that this client library always returns the user, since the user object is returned regardless of the setting. This is one of the reasons we create our own libraries - so that we can change "default behaviour" on the tools we use.

@mrkldshv - the PR I have linked is to solve that

@thepandaguitar
Copy link

thepandaguitar commented Feb 9, 2021

@kiwicopple Ideally supabase.auth.signUp should

  • send you the user object if the user is not signed up.
  • send you a conflict error if the user is already signed up.

Also, running the function twice in a row returns this:

{
  data: null,
  user: null,
  error: Error: Error sending confirmation mail
      at /node_modules/@supabase/gotrue-js/dist/main/lib/fetch.js:23:23
      at processTicksAndRejections (internal/process/task_queues.js:93:5)
}

@kiwicopple
Copy link
Member

Let me set up some more test cases for this. It seems there are a few quirks with GoTrue which we need to cover.

@kiwicopple
Copy link
Member

I've added 3 test suites:

I can confirm that @thepandaguitar's case is throwing a Error sending confirmation mail error, which is a bit obscure. I will see if I can shape this based on the GoTrue server's response headers.

I'm also going to add a session variable to all signUp/signIn funcitons, which will make things easier to reason about:

const { user, session } = supabase.auth.signUp({ email, password })

// with auto confirm on: user and session are both returned
// with auto confirm off: user is returned but session is null

@github-actions
Copy link
Contributor

github-actions bot commented Feb 9, 2021

🎉 This issue has been resolved in version 1.10.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

kiwicopple added a commit to supabase/supabase-js that referenced this issue Feb 9, 2021
@kiwicopple
Copy link
Member

kiwicopple commented Feb 9, 2021

OK all, thanks for the patience:

I will spend some time improving the Auth docs, in particular I think it will be useful to have the return types listed, but that's part of a larger issue which is already open

@thepandaguitar - I created a new issue to track the strange error from GoTrue: #55

@thepandaguitar
Copy link

@kiwicopple Thank you for the quick resolution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation help wanted Extra attention is needed released
Projects
None yet
Development

No branches or pull requests

5 participants