hippware / rn-chat

MIT License
5 stars 0 forks source link

GraphQL userUpdate error: has already been taken #4821

Open aksonov opened 4 years ago

aksonov commented 4 years ago

Error in tinyrobot

Error in MainActivity GraphQL userUpdate error: {"typename":"UserUpdatePayload","messages":[{"typename":"ValidationMessage","message":"has already been taken"}],"successful":false}

View on Bugsnag

Stacktrace

third-party/wocky-client/src/transport/Transport.ts:1365 - 

View full stacktrace

Created by bengtan via Bugsnag

aksonov commented 4 years ago

@miranda Do you have any idea how to reproduce it?

mstidham commented 4 years ago

@aksonov I have no idea, don't recall seeing anything on the Pixel.

bengtan commented 4 years ago

From loggly, this is the request:

["9","59","__absinthe__:control","doc",{"query":"mutation userUpdate($values: UserParams!) {\n userUpdate(input: {values: $values}) {\n successful\n messages {\n message\n __typename\n }\n __typename\n }\n}\n","variables":{"values":{"handle":"text","firstName":"text","lastName":"text","clientData":"{\"sharePresencePrimed\":true,\"guestOnce\":false,\"onboarded\":true,\"hidden\":{\"enabled\":false,\"expires\":null}}"}}}]

and this is the response:

["9","59","__absinthe__:control","phx_reply",{"response":{"data":{"userUpdate":{"__typename":"UserUpdatePayload","messages":[{"__typename":"ValidationMessage","message":"has already been taken"}],"successful":false}}},"status":"ok"}]

The account which issued this request is bbf2b279-d0b1-473d-b35a-a707e8b19806 and has handle ext (according to bugsnag).

From the request above, this account tried to change it's handle to text.

There is already an existing account 2fdcbf01-de3c-4339-a3fa-2029566c09f2 with handle text. So probably, the server returned an error because the handle text already exists.


Suggestion to reproduce:

  1. Get an existing Staging account and try to change it's handle to text

I tried this but the app won't let me save. It says 'Handle not available'.

Yet, somehow, in the bugsnag, the app issued a request to change to an already existing handle. So client side validation was bypassed somehow.

So, next, try this:

  1. Disable client side validation. Then: Get an existing Staging account and try to change it's handle to text
bengtan commented 4 years ago

Oh, I did manage to reproduce it.

Here are the steps:

  1. Using a Staging account, go to the 'Edit Profile' page.
  2. Change the handle to text (or some other existing handle). The form will say 'Handle not available'.
  3. Ignore the 'Handle not available' message and tap on Save.

Surprisingly, the app doesn't crash even though bugsnag says it's an Unhandled error. *shrugs*

aksonov commented 4 years ago

Yes, we just need to disable Save button for invalid input. Ugh, it looks quite complex - MyAccount.tsx 'Save' button doesn't depend from form validation status and it is bad.

bengtan commented 4 years ago

Alternatively ... the app should still handle the situation when it tries to userUpdate a duplicate handle anyway ... because there will always be a time window/race-condition when it could happen.

And if the app handles this error, then it's okay if the Save button isn't disabled for invalid input.

aksonov commented 4 years ago

@bengtan You are right, we could do that first and improve validation later, in other ticket. For now I've done following Simulator Screen Shot - iPhone 8 - 2020-03-06 at 18 23 51

bengtan commented 4 years ago

Whoops.

I just realised my commit c6e4cc5d interferes with @aksonov's changes for this ticket.

I'm thinking of reverting c6e4cc5d ...

bengtan commented 4 years ago

This isn't QA-able until after I revert c6e4cc5.

mstidham commented 4 years ago

Staging Version: 4.41.0 Even though this isn't QA-able it worked great for me. I got the popup error when using an already used handle.

bengtan commented 4 years ago

This isn't QA-able until after I revert c6e4cc5.

Double whoops. It turns out c6e4cc5 doesn't interfere with this ticket.

@mstidham:

If you want to QA this, then ... If you can get the popup error to occur, that's good enough.