ThexXTURBOXx / flutter_web_auth_2

Flutter plugin for authenticating a user with a web service
https://pub.dev/packages/flutter_web_auth_2
MIT License
51 stars 50 forks source link

[Bug]: Seeing crashes during OAuth flow when network errors occur #84

Closed btrautmann closed 9 months ago

btrautmann commented 10 months ago

Description

I've had a few crashes reported with this package in the stacktrace:

Crashed: com.apple.main-thread
0  flutter_web_auth_2             0x54d8 $s18flutter_web_auth_226SwiftFlutterWebAuth2PluginC6handle_6resultySo0E10MethodCallC_yypSgctFy10Foundation3URLVSg_s5Error_pSgtcfU_ + 1452
1  flutter_web_auth_2             0x5984 $s10Foundation3URLVSgs5Error_pSgIegng_So5NSURLCSgSo7NSErrorCSgIeyByy_TR + 192
2  AuthenticationServices         0x24d74 <redacted> + 592
3  flutter_web_auth_2             0x70c4 $s18flutter_web_auth_226SwiftFlutterWebAuth2PluginC6handle_6resultySo0E10MethodCallC_yypSgctF015$syXlSgIeyBy_ypO7Iegn_TRyXlSgIeyBy_Tf1ncn_nTf4nng_n + 3348
4  flutter_web_auth_2             0x5534 $s18flutter_web_auth_226SwiftFlutterWebAuth2PluginC6handle_6resultySo0E10MethodCallC_yypSgctFTo + 84
5  Flutter                        0x5d61dc (Missing UUID 4c4c441f55553144a1682f45551c1451)
6  Flutter                        0x435a4 (Missing UUID 4c4c441f55553144a1682f45551c1451)
7  libdispatch.dylib              0x2320 _dispatch_call_block_and_release + 32
8  libdispatch.dylib              0x3eac _dispatch_client_callout + 20
9  libdispatch.dylib              0x126a4 _dispatch_main_queue_drain + 928
10 libdispatch.dylib              0x122f4 _dispatch_main_queue_callback_4CF + 44
11 CoreFoundation                 0x98c28 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 16
12 CoreFoundation                 0x7a560 __CFRunLoopRun + 1992
13 CoreFoundation                 0x7f3ec CFRunLoopRunSpecific + 612
14 GraphicsServices               0x135c GSEventRunModal + 164
15 UIKitCore                      0x39d6e8 -[UIApplication _run] + 888
16 UIKitCore                      0x39d34c UIApplicationMain + 340
17 Runner                         0x7904 main + 5 (AppDelegate.swift:5)
18 ???                            0x1ecbcedec (Missing)

In several of the reports, it seems a network request in the oauth flow has thrown a 404:

Screenshot 2023-11-20 at 09 52 35

This package is being used transitively via oauth2_client 3.2.2.

Minimal Reproduction

  1. Depend on oauth_client ^3.2.1
  2. Initiate oauth flow
  3. Have error (404) returned by an endpoint in oauth flow
  4. See error

Exception or Error

See description

Expected Behaviour

The package should gracefully handle network failures.

Screenshots

No response

Additional context

No response

Device

iPhone 14 Pro

OS

iOS 16.5.1

Browser

N/A

Flutter version

3.13.6

flutter_web_auth_2 version

2.2.1

Checklist

ThexXTURBOXx commented 10 months ago

Please see if https://github.com/teranetsrl/oauth2_client/pull/178 fixes these issues. If not, I am open for PRs

btrautmann commented 10 months ago

Thanks, I'll give that a shot and see if I see any new instances of the crash. If you prefer to close this and have me re-open if I have more crashes, feel free to do so!

ThexXTURBOXx commented 10 months ago

You're welcome :) I think, I will leave this open for the time being. Maybe other people have the same problem and are jumping on the same bandwagon.

btrautmann commented 9 months ago

I can't say whether this was working (I think I had a regression after a while but not 100% sure), but unfortunately this did break my login functionality because of this PR (specifically the introduction of silentAuth). It makes it so the JSON can't be deserialized. I had to revert the dependency override for now.

ThexXTURBOXx commented 9 months ago

@rundfunk47 Have you made sure that the other approaches still work as expected? Probably the local storage approach is not functioning correctly anymore, could you check that please? Otherwise, I might have to disable your changes for now

rundfunk47 commented 9 months ago

@ThexXTURBOXx @btrautmann

If I understand this correctly, the issue is that silentAuth: json['silentAuth'] does not default to false when deserialising, but crashes because there is no value? Or are you referring to another issue @ThexXTURBOXx ?

ThexXTURBOXx commented 9 months ago

@btrautmann Is that the issue? I thought you were maybe referring to something else?

btrautmann commented 9 months ago

Sorry, I'm a little confused. Here's the series of events as I understand it:

  1. I was experiencing errors that occurred (to the best of my understanding) when users experienced network errors in the OAuth flow, so I opened this issue.
  2. Per the suggestion above, I used a dependency override to point my dependency on oauth2_client at https://github.com/ThexXTURBOXx/oauth2_client.git, which IIRC used a more recent version of flutter_web_auth2
  3. From what I can tell looking at Crashlytics, this seemed to work (I don't see any recent errors like the original one reported)
  4. I recently upgraded Flutter and in doing so, ran a dart pub upgrade, which bumped flutter_web_auth_2 from 3.0.4 to 3.1.0
  5. The bump from 3.0.4 to 3.1.0 introduced a breaking change which broke my login flow (I found out after the app was already in production). See the image of the stacktrace below.
  6. I identified this PR as the culprit.

Due to the above, I removed the dependency override as I wanted to fix the app in production. This resolved the issue (since I'm now using the older version of flutter_web_auth2.

Screenshot 2023-12-22 at 07 12 32

rundfunk47 commented 9 months ago

so I'm assuming changing:

FlutterWebAuth2Options.fromJson(Map<String, dynamic> json)
      : this(
          preferEphemeral: json['preferEphemeral'],
          debugOrigin: json['debugOrigin'],
          intentFlags: json['intentFlags'],
          windowName: json['windowName'],
          timeout: json['timeout'],
          landingPageHtml: json['landingPageHtml'],
          silentAuth: json['silentAuth'],
        );

to

FlutterWebAuth2Options.fromJson(Map<String, dynamic> json)
      : this(
          preferEphemeral: json['preferEphemeral'],
          debugOrigin: json['debugOrigin'],
          intentFlags: json['intentFlags'],
          windowName: json['windowName'],
          timeout: json['timeout'],
          landingPageHtml: json['landingPageHtml'],
          silentAuth: json['silentAuth'] ?? false,
        );

will likely fix the latter issue? I can create a PR for that 🙂

btrautmann commented 9 months ago

Yes, I suspect that will fix the issue :)

ThexXTURBOXx commented 9 months ago

@rundfunk47 That should not be the problem as the preferEphemeral bool works without any problems. @btrautmann Could you please share any exceptions or logs? If that really is the problem, then there should be some exceptions

ThexXTURBOXx commented 9 months ago

Maybe, this might be a fix anyway. I will try out soon!

ThexXTURBOXx commented 9 months ago

I just uploaded 3.1.1, which should fix this issue. Please confirm whether this is the case @btrautmann

btrautmann commented 9 months ago

Could you please share any exceptions or logs? If that really is the problem, then there should be some exceptions

I posted the logs/Exception above. I'll give the new version a shot some time today.

ThexXTURBOXx commented 9 months ago

Yes, sorry, I overlooked them at first, but found them afterwards. They are related to a problem I fixed once, but reintroduced with 3.1.0

btrautmann commented 9 months ago

Just checked, and yeah, that fixed it on both platforms 👍 Thank you for the quick turnaround!

ThexXTURBOXx commented 9 months ago

Perfect, thank you very much for the feedback! :)