[Auth] Fix error typing and bring MFA error into alignment with the current API standard #5616

sam-gc · 2021-10-14T16:41:07Z

Specifically, MultiFactorError was putting fields on the error itself instead of error.customData. The main AuthError was doing the right thing in the code, but the public typing was wrong. With this PR both the typing and the implementation should be consistent using the new API standard (by putting everything on error.customData). Thanks @Pablion for catching the issue. The reference docs will update automatically with this PR.

* fix interface AuthError change interface AuthError to correct structure * remove `declare`

changeset-bot · 2021-10-14T16:41:10Z

🦋 Changeset detected

Latest commit: b4ae127

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@firebase/auth	Patch
@firebase/auth-compat	Patch
firebase	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

google-oss-bot · 2021-10-14T16:58:54Z

Binary Size Report

Affected SDKs

@firebase/auth/cordova

Type Base (703ef77) Head (3da0d9a) Diff

browser 178 kB 179 kB +132 B (+0.1%)

module 178 kB 179 kB +132 B (+0.1%)
@firebase/auth/internal

Type Base (703ef77) Head (3da0d9a) Diff

browser 163 kB 163 kB +144 B (+0.1%)

esm5 211 kB 211 kB +153 B (+0.1%)

main 178 kB 178 kB +132 B (+0.1%)

module 163 kB 163 kB +144 B (+0.1%)
@firebase/auth/react-native

Type Base (703ef77) Head (3da0d9a) Diff

browser 143 kB 143 kB +22 B (+0.0%)

module 143 kB 143 kB +22 B (+0.0%)

`@firebase/firestore`

Type	Base (`703ef77`)	Head (`3da0d9a`)	Diff
browser	225 kB	225 kB	+253 B (+0.1%)
esm5	282 kB	282 kB	+273 B (+0.1%)
main	512 kB	423 kB	-89.4 kB (-17.4%)
module	225 kB	225 kB	+253 B (+0.1%)
react-native	225 kB	225 kB	+253 B (+0.1%)

@firebase/firestore-lite

Type Base (703ef77) Head (3da0d9a) Diff

main 147 kB 123 kB -23.8 kB (-16.1%)

`firebase`

Type	Base (`703ef77`)	Head (`3da0d9a`)	Diff
firebase-auth-compat.js	122 kB	122 kB	+37 B (+0.0%)
firebase-auth-cordova.js	459 kB	460 kB	+399 B (+0.1%)
firebase-auth-react-native.js	430 kB	430 kB	+43 B (+0.0%)
firebase-auth.js	409 kB	410 kB	+311 B (+0.1%)
firebase-compat.js	749 kB	749 kB	+288 B (+0.0%)
firebase-firestore-compat.js	277 kB	277 kB	+251 B (+0.1%)
firebase-firestore.js	762 kB	763 kB	+792 B (+0.1%)

Test Logs

Base (703ef779): https://github.com/firebase/firebase-js-sdk/actions/runs/1339339863
Head (3da0d9a7): https://github.com/firebase/firebase-js-sdk/actions/runs/1360339058

google-oss-bot · 2021-10-14T17:23:06Z

Size Analysis Report

Affected Products

@firebase/auth
- EmailAuthCredential
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  31.9 kB
  31.9 kB
  +1 B (+0.0%)
  size-with-ext-deps
  43.8 kB
  43.8 kB
  +1 B (+0.0%)
- EmailAuthProvider
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  33.3 kB
  33.3 kB
  +1 B (+0.0%)
  size-with-ext-deps
  45.5 kB
  45.5 kB
  +1 B (+0.0%)
- FacebookAuthProvider
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  34.9 kB
  34.9 kB
  +1 B (+0.0%)
  size-with-ext-deps
  46.8 kB
  46.8 kB
  +1 B (+0.0%)
- GithubAuthProvider
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  34.9 kB
  34.9 kB
  +1 B (+0.0%)
  size-with-ext-deps
  46.8 kB
  46.8 kB
  +1 B (+0.0%)
- GoogleAuthProvider
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  34.9 kB
  34.9 kB
  +1 B (+0.0%)
  size-with-ext-deps
  46.8 kB
  46.8 kB
  +1 B (+0.0%)
- OAuthCredential
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  32.0 kB
  32.0 kB
  +1 B (+0.0%)
  size-with-ext-deps
  43.9 kB
  43.9 kB
  +1 B (+0.0%)
- OAuthProvider
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  35.9 kB
  35.9 kB
  +1 B (+0.0%)
  size-with-ext-deps
  47.7 kB
  47.7 kB
  +1 B (+0.0%)
- PhoneAuthCredential
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  32.0 kB
  32.0 kB
  +1 B (+0.0%)
  size-with-ext-deps
  43.9 kB
  43.9 kB
  +1 B (+0.0%)
- PhoneAuthProvider
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  33.7 kB
  33.7 kB
  +1 B (+0.0%)
  size-with-ext-deps
  45.6 kB
  45.6 kB
  +1 B (+0.0%)
- SAMLAuthProvider
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  36.2 kB
  36.2 kB
  +1 B (+0.0%)
  size-with-ext-deps
  48.1 kB
  48.1 kB
  +1 B (+0.0%)
- TwitterAuthProvider
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  34.9 kB
  34.9 kB
  +1 B (+0.0%)
  size-with-ext-deps
  46.8 kB
  46.8 kB
  +1 B (+0.0%)
- createUserWithEmailAndPassword
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  30.9 kB
  30.9 kB
  +1 B (+0.0%)
  size-with-ext-deps
  42.8 kB
  42.8 kB
  +1 B (+0.0%)
- getAuth
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  61.5 kB
  61.6 kB
  +79 B (+0.1%)
  size-with-ext-deps
  74.0 kB
  74.1 kB
  +79 B (+0.1%)
- getMultiFactorResolver
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  32.5 kB
  32.5 kB
  -30 B (-0.1%)
  size-with-ext-deps
  44.4 kB
  44.4 kB
  -30 B (-0.1%)
- getRedirectResult
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  37.3 kB
  37.4 kB
  +58 B (+0.2%)
  size-with-ext-deps
  49.2 kB
  49.3 kB
  +58 B (+0.1%)
- linkWithPhoneNumber
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  34.5 kB
  34.5 kB
  +1 B (+0.0%)
  size-with-ext-deps
  46.3 kB
  46.4 kB
  +1 B (+0.0%)
- linkWithPopup
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  41.0 kB
  41.0 kB
  -21 B (-0.1%)
  size-with-ext-deps
  52.9 kB
  52.9 kB
  -21 B (-0.0%)
- linkWithRedirect
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  36.2 kB
  36.2 kB
  +1 B (+0.0%)
  size-with-ext-deps
  48.1 kB
  48.1 kB
  +1 B (+0.0%)
- reauthenticateWithCredential
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  31.4 kB
  31.4 kB
  -22 B (-0.1%)
  size-with-ext-deps
  43.2 kB
  43.2 kB
  -22 B (-0.1%)
- reauthenticateWithPhoneNumber
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  35.0 kB
  35.0 kB
  -21 B (-0.1%)
  size-with-ext-deps
  46.9 kB
  46.9 kB
  -21 B (-0.0%)
- reauthenticateWithPopup
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  41.0 kB
  41.0 kB
  -21 B (-0.1%)
  size-with-ext-deps
  52.9 kB
  52.9 kB
  -21 B (-0.0%)
- reauthenticateWithRedirect
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  35.9 kB
  35.9 kB
  +1 B (+0.0%)
  size-with-ext-deps
  47.8 kB
  47.8 kB
  +1 B (+0.0%)
- signInAnonymously
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  31.0 kB
  31.0 kB
  +1 B (+0.0%)
  size-with-ext-deps
  42.9 kB
  42.9 kB
  +1 B (+0.0%)
- signInWithCredential
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  31.2 kB
  31.1 kB
  -22 B (-0.1%)
  size-with-ext-deps
  43.0 kB
  43.0 kB
  -22 B (-0.1%)
- signInWithCustomToken
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  30.9 kB
  30.9 kB
  +1 B (+0.0%)
  size-with-ext-deps
  42.8 kB
  42.8 kB
  +1 B (+0.0%)
- signInWithEmailAndPassword
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  34.8 kB
  34.8 kB
  -21 B (-0.1%)
  size-with-ext-deps
  46.9 kB
  46.9 kB
  -21 B (-0.0%)
- signInWithEmailLink
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  35.0 kB
  35.0 kB
  -21 B (-0.1%)
  size-with-ext-deps
  47.1 kB
  47.1 kB
  -21 B (-0.0%)
- signInWithPhoneNumber
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  34.8 kB
  34.8 kB
  -21 B (-0.1%)
  size-with-ext-deps
  46.7 kB
  46.7 kB
  -21 B (-0.0%)
- signInWithPopup
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  41.0 kB
  41.0 kB
  -21 B (-0.1%)
  size-with-ext-deps
  52.9 kB
  52.9 kB
  -21 B (-0.0%)
- signInWithRedirect
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  35.6 kB
  35.6 kB
  +1 B (+0.0%)
  size-with-ext-deps
  47.5 kB
  47.5 kB
  +1 B (+0.0%)
- browserPopupRedirectResolver
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  53.8 kB
  53.9 kB
  +79 B (+0.1%)
  size-with-ext-deps
  65.9 kB
  65.9 kB
  +79 B (+0.1%)
@firebase/firestore
- addDoc
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  103 kB
  103 kB
  +230 B (+0.2%)
  size-with-ext-deps
  152 kB
  152 kB
  +230 B (+0.2%)
- deleteDoc
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  94.3 kB
  94.5 kB
  +230 B (+0.2%)
  size-with-ext-deps
  143 kB
  143 kB
  +230 B (+0.2%)
- enableIndexedDbPersistence
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  139 kB
  139 kB
  +230 B (+0.2%)
  size-with-ext-deps
  188 kB
  188 kB
  +230 B (+0.1%)
- enableMultiTabIndexedDbPersistence
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  170 kB
  171 kB
  +253 B (+0.1%)
  size-with-ext-deps
  220 kB
  220 kB
  +253 B (+0.1%)
- executeWrite
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  93.4 kB
  93.7 kB
  +230 B (+0.2%)
  size-with-ext-deps
  142 kB
  142 kB
  +230 B (+0.2%)
- getDoc
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  119 kB
  119 kB
  +230 B (+0.2%)
  size-with-ext-deps
  168 kB
  168 kB
  +230 B (+0.1%)
- getDocFromServer
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  119 kB
  119 kB
  +230 B (+0.2%)
  size-with-ext-deps
  168 kB
  168 kB
  +230 B (+0.1%)
- getDocs
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  121 kB
  121 kB
  +230 B (+0.2%)
  size-with-ext-deps
  169 kB
  170 kB
  +230 B (+0.1%)
- getDocsFromServer
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  121 kB
  121 kB
  +230 B (+0.2%)
  size-with-ext-deps
  169 kB
  169 kB
  +230 B (+0.1%)
- onSnapshot
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  121 kB
  121 kB
  +230 B (+0.2%)
  size-with-ext-deps
  170 kB
  170 kB
  +230 B (+0.1%)
- onSnapshotsInSync
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  111 kB
  111 kB
  +230 B (+0.2%)
  size-with-ext-deps
  160 kB
  160 kB
  +230 B (+0.1%)
- setDoc
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  102 kB
  103 kB
  +230 B (+0.2%)
  size-with-ext-deps
  151 kB
  151 kB
  +230 B (+0.2%)
- updateDoc
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  103 kB
  103 kB
  +230 B (+0.2%)
  size-with-ext-deps
  152 kB
  152 kB
  +230 B (+0.2%)
- writeBatch
  Size Table
  Type Base (703ef77) Head (3da0d9a) Diff
  size
  105 kB
  105 kB
  +230 B (+0.2%)
  size-with-ext-deps
  154 kB
  154 kB
  +230 B (+0.1%)

Test Logs

lisajian

LGTM, just a couple nits

packages/auth/src/core/strategies/credential.test.ts

packages/auth/src/core/user/reauthenticate.test.ts

packages/auth/src/model/public_types.ts

sam-gc · 2021-10-14T20:04:25Z

Adding @Feiyang1 and @hsubox76 for approval for changes to api reivew

packages/auth/src/model/public_types.ts

PabloLION · 2021-10-18T20:40:40Z

Comment in English is not my advantage, I prefer the new comments.
And dont forget to close #5599 after this please.

Co-authored-by: Kevin Cheung <kevinthecheung@users.noreply.github.com>

.changeset/fresh-ways-think.md

PabloLION · 2021-10-19T19:52:00Z

@sam-gc Thank for fixting this! I think this closes #5599. Please confirm.
By the way, the document I mentioned there seems not fixed? Should I create a new issue?

Also this part seems to be outdated(v8 SDK yet shown up in a v9 page)
like auth.signInWithPopup(OAuthprovider) should be signInWithPopup(authInstance,OAuthProvider), so should auth.signInWithEmailAndPassword(email, password) (first arg: auth instance)... etc.

PabloLION and others added 2 commits October 14, 2021 09:19

fix interface AuthError (#5609)

39dd81f

* fix interface AuthError change interface AuthError to correct structure * remove `declare`

Refactor MFA error to properly follow the new format

522e2e7

sam-gc assigned lisajian and yuchenshi Oct 14, 2021

sam-gc requested review from avolkovi, Feiyang1, hiranya911, hsubox76, lisajian and yuchenshi as code owners October 14, 2021 16:41

google-cla bot added the cla: yes label Oct 14, 2021

Add changeset

a826c7e

sam-gc requested a review from egilmorez as a code owner October 14, 2021 16:54

google-oss-bot added the doc-changes PRs that affect docs label Oct 14, 2021

github-actions bot assigned egilmorez Oct 14, 2021

lisajian approved these changes Oct 14, 2021

View reviewed changes

packages/auth/src/core/strategies/credential.test.ts Show resolved Hide resolved

packages/auth/src/core/user/reauthenticate.test.ts Show resolved Hide resolved

yuchenshi approved these changes Oct 14, 2021

View reviewed changes

packages/auth/src/model/public_types.ts Show resolved Hide resolved

Add doc comment to MultiFactorError.customData

4ef325a

sam-gc assigned Feiyang1 and hsubox76 Oct 14, 2021

egilmorez requested a review from kevinthecheung October 18, 2021 18:36

kevinthecheung approved these changes Oct 18, 2021

View reviewed changes

Apply suggestions from code review

821920f

Co-authored-by: Kevin Cheung <kevinthecheung@users.noreply.github.com>

Feiyang1 approved these changes Oct 19, 2021

View reviewed changes

.changeset/fresh-ways-think.md Show resolved Hide resolved

Update changeset to patch

b4ae127

sam-gc merged commit 69ff8eb into master Oct 19, 2021

sam-gc deleted the auth_error_type_fix branch October 19, 2021 19:38

This was referenced Oct 19, 2021

Version Packages #5639

Merged

Version Packages #5655

Merged

PabloLION mentioned this pull request Nov 3, 2021

Incoherent data structure on AuthError #5599

Closed

2 tasks

firebase locked and limited conversation to collaborators Nov 19, 2021

[Auth] Fix error typing and bring MFA error into alignment with the current API standard #5616

[Auth] Fix error typing and bring MFA error into alignment with the current API standard #5616

Conversation

sam-gc commented Oct 14, 2021

changeset-bot bot commented Oct 14, 2021 • edited Loading

🦋 Changeset detected

google-oss-bot commented Oct 14, 2021 • edited Loading

Binary Size Report

Affected SDKs

@firebase/auth/cordova

@firebase/auth/internal

@firebase/auth/react-native

@firebase/firestore

@firebase/firestore-lite

firebase

Test Logs

google-oss-bot commented Oct 14, 2021 • edited Loading

Size Analysis Report

Affected Products

@firebase/auth

EmailAuthCredential

EmailAuthProvider

FacebookAuthProvider

GithubAuthProvider

GoogleAuthProvider

OAuthCredential

OAuthProvider

PhoneAuthCredential

PhoneAuthProvider

SAMLAuthProvider

TwitterAuthProvider

createUserWithEmailAndPassword

getAuth

getMultiFactorResolver

getRedirectResult

linkWithPhoneNumber

linkWithPopup

linkWithRedirect

reauthenticateWithCredential

reauthenticateWithPhoneNumber

reauthenticateWithPopup

reauthenticateWithRedirect

signInAnonymously

signInWithCredential

signInWithCustomToken

signInWithEmailAndPassword

signInWithEmailLink

signInWithPhoneNumber

signInWithPopup

signInWithRedirect

browserPopupRedirectResolver

@firebase/firestore

addDoc

deleteDoc

enableIndexedDbPersistence

enableMultiTabIndexedDbPersistence

executeWrite

getDoc

getDocFromServer

getDocs

getDocsFromServer

onSnapshot

onSnapshotsInSync

setDoc

updateDoc

writeBatch

Test Logs

lisajian left a comment

Choose a reason for hiding this comment

sam-gc commented Oct 14, 2021

PabloLION commented Oct 18, 2021

PabloLION commented Oct 19, 2021 • edited Loading

changeset-bot bot commented Oct 14, 2021 •

edited

Loading

google-oss-bot commented Oct 14, 2021 •

edited

Loading

`@firebase/auth/cordova`

`@firebase/auth/internal`

`@firebase/auth/react-native`

`@firebase/firestore`

`@firebase/firestore-lite`

`firebase`

google-oss-bot commented Oct 14, 2021 •

edited

Loading

`@firebase/auth`

`EmailAuthCredential`

`EmailAuthProvider`

`FacebookAuthProvider`

`GithubAuthProvider`

`GoogleAuthProvider`

`OAuthCredential`

`OAuthProvider`

`PhoneAuthCredential`

`PhoneAuthProvider`

`SAMLAuthProvider`

`TwitterAuthProvider`

`createUserWithEmailAndPassword`

`getAuth`

`getMultiFactorResolver`

`getRedirectResult`

`linkWithPhoneNumber`

`linkWithPopup`

`linkWithRedirect`

`reauthenticateWithCredential`

`reauthenticateWithPhoneNumber`

`reauthenticateWithPopup`

`reauthenticateWithRedirect`

`signInAnonymously`

`signInWithCredential`

`signInWithCustomToken`

`signInWithEmailAndPassword`

`signInWithEmailLink`

`signInWithPhoneNumber`

`signInWithPopup`

`signInWithRedirect`

`browserPopupRedirectResolver`

`@firebase/firestore`

`addDoc`

`deleteDoc`

`enableIndexedDbPersistence`

`enableMultiTabIndexedDbPersistence`

`executeWrite`

`getDoc`

`getDocFromServer`

`getDocs`

`getDocsFromServer`

`onSnapshot`

`onSnapshotsInSync`

`setDoc`

`updateDoc`

`writeBatch`

PabloLION commented Oct 19, 2021 •

edited

Loading