Guard against NullPointerException when getting Credentials from Json

[bennycao]

Changes

This change adds code to guard against possible NullPointerException thrown as a result of f being null in the JsonRequiredTypeAdaptor

Solves https://github.com/auth0/Auth0.Android/issues/676

Testing

Checklist

• [ ] I have read the Auth0 general contribution guidelines

• [ ] I have read the Auth0 Code of Conduct

• [ ] All existing and new tests complete without errors

[poovamraj]

@bennycao I'd rather prefer us to check if the field is null here like below

if (f!= null && f.getAnnotation(JsonRequired.class) != null) {

Instead of catching the exception. It feels much cleaner and the check makes more sense. Can you try this solution out?

[bennycao]

@bennycao I'd rather prefer us to check if the field is null here like below

if (f!= null && f.getAnnotation(JsonRequired.class) != null) {

Instead of catching the exception. It feels much cleaner and the check makes more sense. Can you try this solution out?

@poovamraj the question i would have is if f is null, what actually happens ? I don't have intimate enough knowledge of the code. Yes it would stop the exception but not sure what the side effect would be. I did ask this exact question in the issue i raised https://github.com/auth0/Auth0.Android/issues/676#issuecomment-1685422498 .

Raising an exception as i see it allows the consumer to handle it, not just side step the issue.

[poovamraj]

@bennycao yes if the required value like expiresAt is not present for constructing the object, there will be an error thrown, but since this is fixing an issue that no one is able to reproduce, this is the only change that will introduce no new changes to the public API but at the same time solve your issue. If making this change introduces the new issue in your error logs, atleast you will be able to further debug it.

[bennycao]

@bennycao yes if the required value like expiresAt is not present for constructing the object, there will be an error thrown, but since this is fixing an issue that no one is able to reproduce, this is the only change that will introduce no new changes to the public API but at the same time solve your issue. If making this change introduces the new issue in your error logs, atleast you will be able to further debug it.

@poovamraj i've made the update as requested.

[bennycao]

@poovamraj Any chance we can get this looked thanks ?

[bennycao]

@poovamraj thanks for the approval. When do we think a new release with this fix be available ? In terms of build status, i'm not sure what is happening there. But let me know if i need to do anything. Thanks

[bennycao]

@poovamraj any movement on getting this into a release ? we would like to close this out asap. Thanks

[poovamraj]

@bennycao you can go ahead with merging the PR after ensuring all checks are passed. We can cut a patch release once this is merged.

[bennycao]

@bennycao you can go ahead with merging the PR after ensuring all checks are passed. We can cut a patch release once this is merged.

@poovamraj it seems some build steps require authorizations to run. And i don't have rights to merge. Can you please authorize these ?