search

line / flutter_line_sdk

A Flutter plugin that lets developers access LINE's native SDKs in Flutter apps with Dart.
https://developers.line.biz/
Apache License 2.0
213 stars 43 forks source link

Don't throw error even if current token is null #9

Closed yoshimin closed 4 years ago

yoshimin commented 4 years ago

In iOS, currentAccessToken does not throw error even if token is null, but currently throw in android. I want the same behavior on both OS.

CLAassistant commented 4 years ago

CLA assistant check
All committers have signed the CLA.

onevcat commented 4 years ago

@plateaukao How do you think about it?

IMO, the non-existing token should not be an error, but return a null to the caller. So I think making Android behavior aligning with iOS should be a nicer choice.

But I am afraid it should not be a result.error(null), but a result.success(null). Can you review and confirm it?

onevcat commented 4 years ago

The error case is corresponding to these lines in our Android SDK: https://git.linecorp.com/LINE-Client/linesdk-android/blob/v5.0/line-sdk/src/main/java/com/linecorp/linesdk/api/internal/LineApiClientImpl.java#L150-L154

All internal exceptions are caught inside the AccessTokenCache.getAccessToken method. We cannot tell the difference between EncryptionException from the non-existing token. I think it would be fine to ignore the errors and classify all the retrieving exceptions as a "non-existing token" case.

As above, we have two choices in LineSdkWrapper.kt (https://github.com/line/flutter_line_sdk/pull/9/files#diff-3bb5f5fc7df7a490be803e132fd9c2bd) :

  1. Simply treat all !lineApiResponse.isSuccess cases as no token existing, and call result.success(null) to let the caller know it.
  2. Check the error type of lineApiResponse, if it is "The cached access token does not exist." as declared here (I really suspect there is a chance to break it unexpectedly in future...), we call result.success(null). For other error cases (there is no such one currently), we call result.error(error).

Option 1 is enough to me to solve current situation. But option 2 is more future-oreintated and can handle and propagate real errors to caller.