[Proposal] Expose better error code

[timnew]

Problem

Currently, when AppAuth failed, the plugin throws PlatformException. The code of PlatformException is hardcoded to a certain value based on which method the app called, which isn't very useful. As the app should have know which API is called. Instead the actual useful information isn't passed up.

For example, if app calls authorizeAndExchangeCode, and then user canceled the web view popped up.

On iOS:

PlatformException(authorize_and_exchange_code_failed, Failed to authorize: The operation couldn’t be completed. (org.openid.appauth.general error -3.), null, null)

The code authorize_and_exchange_code_failed is useless as we have known we called authorizeAndExchangeCode.

The useful bit is actually org.openid.appauth.general and -3, which is the code and domain of the NSError underneath, but it is mixed in the message that passed up.

on Android:

Error: PlatformException(authorize_and_exchange_code_failed, Failed to authorize: [error: null, description: User cancelled flow], null, null)

Again authorize_and_exchange_code_failed is not helpful. User cancelled flow is the one useful. And we missed some extra information here.

Proposal

When throw PlatformException from plugin, rather than returns hard coded thing that bind to the calling method. Expose the more detail from underneath error.

For iOS: Use the concat the NSError's domain and code as the PlatformException error code. So it would return org.openid.appauth.general -3 instead of authorize_and_exchange_code_failed as code.

And for Android: The AppAuth would throws AuthorizationException, which contains type and code. return the type and code accordingly.

[MaikuB]

A similar thing has been mentioned before and I was mentioning that PRs are welcome on it. I'd likely not get it myself so if you can contribute then that would be great. One concern here is if both platforms even use the same error code. If they don't then this becomes messy as it requires either the consumer of the plugin to handle platform-specific error codes or have a layer within the plugin to map error platform's error code to something consistent across all platforms. The latter could take a fair bit of work to do and requires keeping an eye on new errors added to be releases of the AppAuth iOS and Android SDKs

[timnew]

@MaikuB Check PR https://github.com/MaikuB/flutter_appauth/pull/395

[MaikuB]

Thanks, saw the PR and it does look like it has the issue I was referring to. You can reproduce what I mean by running the example app, select "Sign in with auto code exchange", wait for modal browser to open and cancel. Should find that the error code is different on Android and iOS/macOS as they use different error codes. Note that I agree that having more concrete error codes would be more useful yet but ideally the error code should be same regardless of the platform so that there isn't a need to write code to handle different error codes on each platform. Do you know of a way to achieve this? This would be more errors that don't relate to any RFC but were made the developers/maintainers of the SDKs. This is what I meant by how this may require some mapping within the plugin to achieve though would require digging through the internals of each SDK to see what scenarios they handle. It may be possible that some error codes are specific to one platform and aren't used by the other...

[timnew]

Yep I aware that the error code would be different, in fact the PR doesn't try to unify the error code too. It just expose the native error code to the app.

And I think it is okay to have different error code. As by the end of the day, app dev would have better idea about which platform they care about than the lib dev does. If the they just need to deal with the certain errors raised by the native lib.

So the idea of the PR is to expose the detail of the native error to the flutter app, then the app dev would have a chance to deal with them.

[juandelgado]

have a layer within the plugin to map error platform's error code to something consistent across all platforms

@MaikuB As a selfish user of the library, this is definitely my expectation. At the end of the day, platform abstraction is why I use Flutter and libraries like this. Mind you, I do understand that this is not easy, but again, it's why I've chosen Flutter and its ecosystem: one codebase to rule them all.

Now, trying to be pragmatic, any step towards increasing the ability to provide a better experience to our users would be appreciated, which is why I think @timnew's PR helps, even if I have to do the (ugly) mapping of platform specifc errors on my side of the app instead of having the library to do it.

Just for the record, in my case, the reason I came looking for this is the ability to tell a user when the login failed because the identity provider is down or cannot be contacted. Right now I don't see a way of doing this, since all I get back is a PlatformException(authorize_and_exchange_code_failed), which is also raised when the user purposefully cancels the login flow.

Thanks.

[timnew]

In fact, I had looked into the feasibility to unify the error, actually tried to do it at very beginning. But I ended up with current solution without touching them, and here is why:

1. There are 2 types of errors from the lib, offline ones and server ones. Server ones might share the similar setup, but local ones are very very different. Unifying them is a quite big and heavy job, it is kind of defy the idea as a wrapper.
2. Mapping would introduce plenty lib specific document, so developer would be ended up reading all 3 documents, rather than just one.
3. Adding a mapping layer would introduce more coupling to the underneath libs, any changes on the error codes on the underneath lib would break this lib. But in 90% of time, we don't need those granular details, that's why most of users of this lib won't complain about it. In general, I don't think the effort and benefits worth the effort. As a tradeoff, exposing native error could should be enough, if someone really cares about a certain low level error, they can use the native error code by themselves.

[sebastianbuechler]

I just came across the iOS exception org.openid.appauth.general error -3. and to be honest it's not really helpful. Is there an overview of what exception we can expect in which cases on iOS and android?

[MaikuB]

@sebastianbuechler you would need to check the docs for each platform's SDKs and understand where you're coming. This is why I proposed that ideally they should map to something consistent. not something I have time to do so happy for others in the community like yourself to contribute via a PR