Repository navigation
[V3] ios login errors from loginWithEmailAndPassword coming back as undefined #288
Description
Hi so I'm using the latest v3 branch like this:
export const loginWithEmail = ({ email, password }) =>
((dispatch: Dispatch) => {
console.log(email, password);
dispatch({ type: BEGIN_EMAIL_LOGIN });
firestack.auth()
.signInWithEmailAndPassword(email, password)
.then((user) => {
console.log('firebase sign in succesful: ', user)
dispatch({
type: FIREBASE_LOGIN_SUCCESFUL,
});
})
.catch(err => console.log('error logging into firebase ', err));
});
which works if I provide the right email and password, but any error I get back is just coming back as an object with code of "" and message of undefined:
once I have some error messages to go off I'm finally going to get around to making that error code nromalising module Ive been promising forever, but I need an error code haha
Activity
jsbranco commentedon Mar 6, 2017
@SamMatthewsIsACommonName , I was having the same issue. I have created a pull request to resolve this precise issue : #304 seems to be due to a variable name mismatch.
SamMatthewsIsACommonName commentedon Mar 6, 2017
Hey @jsbranco thanks that's interesting and as you say does work. The error codes are all new yet again haha (possibly the 5th format of error codes we've had so far) We've been working on this kind of on an alternative branch and it was noted that a problem was arising as a result of the '/auth' extension in the lib/modules/auth/index.js file, as referenced here: Salakar#18 . You'll see there I also made a kind of quick and dirty switch module for standardising a message output and if you look there you'll see all the different 'error codes' I've gotten up to this point. It is all kind of a mystery to me haha!
jsbranco commentedon Mar 7, 2017
@SamMatthewIsACommonName I see, so in V3 it shouldnt be using auth/? Or simply the auth/ format was changed but the ios files were not updated as per the latest :)?
I am just asking to check if my change is correct as that fix was honestly from debugging and trying to trace the issue
SamMatthewsIsACommonName commentedon Mar 7, 2017
I'm honestly not sure haha... I mean both kind of work but in different ways (ie they produce completely different error codes)
designorant commentedon Mar 7, 2017
May I vote for #304 to be merged into this fork?
SamMatthewsIsACommonName commentedon Mar 7, 2017
@designorant my personal vote would be for you solution of removing the '/auth' as that produces error codes which are at least documented in some places... i.e the error in the example would be 'code: 17007, message: 'The email....'
The error codes produced by this solution scare me a bit as I can't find mention of them anywhere, whereas there's an exhaustive list available of the '17000' type codes
SamMatthewsIsACommonName commentedon Mar 7, 2017
@jsbranco are you able to try with your solution as well as removing the '/auth', and see what format the codes you get are?
jsbranco commentedon Mar 7, 2017
@SamMatthewsIsACommonName
So I just tried to remove "auth/" as such :
return promisify('signInWithEmail', FirestackAuth)(email, password);And my error code is:
SamMatthewsIsACommonName commentedon Mar 7, 2017
Thanks so that's no change right? Would you be able to keep the '/auth' removed and revert your solution back to how it was and see if you get an error code of 17007? I'm just trying to establish that there is actually some level of consistency
jsbranco commentedon Mar 7, 2017
Sorry @SamMatthewsIsACommonName I just checked.. it was still running the code with auth/ so after fresh restart, the error object has an additional attribute after removing "auth/" although error code is still the same:
Object {error: "createUserWithEmailError", message: "The email address is already in use by another account.", code: "272115"}
I will test it without my changes shortly.
jsbranco commentedon Mar 7, 2017
Without my changes and without auth/ the result is the following:
SamMatthewsIsACommonName commentedon Mar 7, 2017
OK great thanks so much for testing that out. Well that all being the case my vote would still be for the one that produces the 17007 purely because there are resources such as this one documenting all the codes:
http://stackoverflow.com/questions/27420176/list-of-authorisation-errors-with-firebase-login
However the flip side is with that solution the android codes go from being the nice and well documented 'auth/email-in-use' type to 'ERROR_EMAIL_IN_USE' type which are harder to find documentation of.
Edit: although maybe we can make use of the error attribute to find examples, as I was going off of the actual code but the error values seem quite universal. Also sorry to confirm you are getting the 17007 as 'name' prop rather than 'code'?
designorant commentedon Mar 7, 2017
Oh, sorry, I only now realised that we're getting totally different code numbers there.
Since it seems to be related to #252, it would be great to see whether @Salakar and @chrisbianca have any thoughts on what's going on here.
jsbranco commentedon Mar 7, 2017
@SamMatthewsIsACommonName thats right, 17007 is coming as name rather than code. If I assign it as code it will trigger a lowerCase function not applicable in javascript. Because the code is suppose to be a string ( I believe).
The interesting thing is if I debug the error object in Objective C I do see the code as 17007, but I am doing a casting
@"code": [NSString stringWithFormat:@"%d", @([error code])]Which for some reason is giving that 27... code which is not suppose to. As I mentioned earlier, I am not too familiar with Objective-C I just looked into casting in Objective-C and applied it.
EDIT:
Resolved the issue of the code in my fix, the following is the result with both auth/ :
SamMatthewsIsACommonName commentedon Mar 7, 2017
Can you try NSString stringWithFormat:@"%i" as if you were beginning with an integer?
jsbranco commentedon Mar 7, 2017
@SamMatthewsIsACommonName I just did this :
@"code": [NSString stringWithFormat:@"%d", [error code]]and it give sthe 17007 now. I will try to add back auth/ and see what comes upjsbranco commentedon Mar 7, 2017
@SamMatthewsIsACommonName without auth/ and with the fix of casting the result looks better:
SamMatthewsIsACommonName commentedon Mar 7, 2017
Yeah that looks good! Currently without '/auth' I'm getting the code as a number so will be much more consistent to have it as a string
jsbranco commentedon Mar 7, 2017
I shall update my PR with the casting change
Update: PR updated.
jsbranco commentedon Mar 8, 2017
@SamMathewIsACommonName I just updated my PR since I was also having the following issue when using signWithEmailAndPassword :
The commit follows the same changes I did for create a user with email / password :
https://github.com/fullstackreact/react-native-firestack/pull/304/files/3e7490883c8d9b4f0443a8f58f58a91dd097012e..948b882231c1d30cd8b502e2f3ba08b8602aff55
The error output is now like this :
SamMatthewsIsACommonName commentedon Mar 9, 2017
Awesome! I think @auser is responsible for committing pull requests on this branch. Looks good though!
jsbranco commentedon Mar 15, 2017
@auser is there anything else required from my side (update docs or other step) as this my first PR on this library.
Kind Regards
Junio
smkhalsa commentedon Mar 16, 2017
+1