Skip to content

extract errorMessage directly from response when server returns 200 #2542

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

Merged
merged 2 commits into from
Mar 13, 2019

Conversation

Yue-Wang-Google
Copy link
Contributor

@Yue-Wang-Google Yue-Wang-Google commented Mar 13, 2019

Fix #2522.

in case of returnIDPCredential=YES, instead of an error like the following:

[response data:] {
  "error": {
    "code": 400,
    "message": "FEDERATED_USER_ID_ALREADY_LINKED",
    "errors": [
      {
        "message": "FEDERATED_USER_ID_ALREADY_LINKED",
        "domain": "global",
        "reason": "invalid"
      }
    ]
  }
}

the error message is directly enclosed in the response without a structure:

 [response data:] {
  key: value
 ...
  "errorMessage": "FEDERATED_USER_ID_ALREADY_LINKED",
  "kind": "identitytoolkit#VerifyAssertionResponse"
}

…ntial=YES

Fix #2522.

In this case, the server returns an 200 without an error like the following:

```
[response data:] {
  "error": {
    "code": 400,
    "message": "FEDERATED_USER_ID_ALREADY_LINKED",
    "errors": [
      {
        "message": "FEDERATED_USER_ID_ALREADY_LINKED",
        "domain": "global",
        "reason": "invalid"
      }
    ]
  }
}
```

Rather, it is directly enclosed in the response with an errorMessage without a structure:

```
 [response data:] {
  key: value
 ...
  "errorMessage": "FEDERATED_USER_ID_ALREADY_LINKED",
  "kind": "identitytoolkit#VerifyAssertionResponse"
}
```
@Yue-Wang-Google Yue-Wang-Google added this to the M45 milestone Mar 13, 2019
@Yue-Wang-Google Yue-Wang-Google changed the title extract errorMessage directly from response in case of returnIDPCrede… extract errorMessage directly from response when server returns 200 Mar 13, 2019
@Yue-Wang-Google
Copy link
Contributor Author

@mono0926 reported that this fix fully resolved his issue. This is an important fix and needs to go into M45. A changelog update is made in the same PR as well.

@renkelvin when you approve the fix tomorrow morning, feel free to merge it for me as I'll be OOO tomorrow.

Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

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

I'm going to approve and merge this in advance of the release freeze.

@renkelvin Please add a review to confirm when you get in.

@paulb777 paulb777 merged commit 45d2fb2 into master Mar 13, 2019
@paulb777 paulb777 deleted the Yue-Wang-Google-patch-1 branch March 13, 2019 14:38
Copy link
Contributor

@renkelvin renkelvin left a comment

Choose a reason for hiding this comment

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

LGTM

@renkelvin
Copy link
Contributor

@Yue-Wang-Google Talked with backend folks. It seems it deserve some comments to explain why it's handled differently.

@renkelvin renkelvin mentioned this pull request Apr 9, 2019
@firebase firebase locked and limited conversation to collaborators Oct 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Firebase Authentication's error code seems to be broken after upgrading to 5.4.0
4 participants