stripe / stripe-terminal-react-native

React Native SDK for Stripe Terminal
https://stripe.com/docs/terminal/payments/setup-integration?terminal-sdk-platform=react-native
MIT License
103 stars 49 forks source link

Request: Restart the entire native SDK when Javascript app bundle is restarted (Could not execute discoverReaders because the SDK is busy with another command: discoverReaders) #553

Open zameschua opened 10 months ago

zameschua commented 10 months ago

Is your feature request related to a problem? Please describe. For unrelated reasons, I have to restart the React Native app bundle programmatically if the app has been idle for too long. I implement a reader re-connect flow too whenever the app is re-booted, by discovering and connecting to a saved reader serial number. If the javascript app bundle is restarted while running discoverReaders(), the Cancelable reference is lost and I can never cancel discoverReaders() anymore. Subsequent calls to discoverReaders() result in the error Could not execute discoverReaders because the SDK is busy with another command: discoverReaders. Our merchants in production currently have to kill and restart the application whenever this happens.

I run into the exact same issue quite often when developing as well because of the frequent Javascript app bundle restarts (even with hot-reloading disabled). I'll have to kill the entire metro server and re-build the app to get rid of the issues and be able to discoverReaders() again. The workaround is not too bad, just slightly annoying.

Related issues here, but none of the solutions work for me https://github.com/stripe/stripe-terminal-react-native/issues/428 https://github.com/stripe/stripe-terminal-react-native/issues/349

Describe the solution you'd like Restart the entire native SDK when Javascript app bundle is restarted

Describe alternatives you've considered

  1. Try to cancel discovering before the app restarts. Unfortunately the app restarts in the background and I can't be sure that the SDK has initialized by then
  2. Find some way to restart the entire native application instead of just doing a javascript app bundle restart. Unfortunately there's no way to do it currently because iOS doesn't expose that functionality
  3. Stripe SDK to expose a method to re-initialize the SDK. But this is probably going to be more clunky than simply restarting the entire SDK when the Javascript app bundle restarts.
  4. Stripe SDK to persist the cancelable() across app restarts. Probably will be clunkier than just restarting the entire SDK.

Additional context Hope I can get a response quickly because I have live merchants calling me everyday about this issue :(

nazli-stripe commented 10 months ago

@zameschua what's the use case for restarting the app? is it possible to ensure the SDK is in an idle state and any running operation is canceled before restarting?

zameschua commented 10 months ago

Thanks for getting back to me @nazli-stripe

We're building a POS application where the cashiers usually just 'sleep' the RN application without ever closing it. We have to restart the app programmatically at the start of each day to clean up unused memory so the app can continue running smoothly, if not it starts to lag pretty badly.

It might be possible to make sure that the SDK is in an idle state but will require quite a few workarounds because we currently enter the reader reconnection flow when the main app component is mounted and disconnected from the reader, which I'm guessing currently coincides with the application restart.

I'll need to rework the rendering tree so that the app will not start the reader reconnection flow until after the app has restarted, or not restart the app until the stripe reader is in an idle state.

Ideally the native SDK would re-start from a fresh state when the javascript bundle is restarted as well so we don't have to implement the workarounds, and will be much easier in development if we can just do a Javascript bundle reload.

nazli-stripe commented 10 months ago

Gotcha, thanks for the details. Restarting the native SDK on JS refresh is not something we have the bandwidth to effectively support today as it is not super straightforward. Some operations are not cancelable and could fail. Recreating a new instance could leave an orphaned instance that's already connected to a reader, making that reader unavailable in discovery. So we feel more comfortable if the integration handled these states. I'll keep this open as a feature request and as always welcome any PRs if a user wants to take a stab at this

zameschua commented 10 months ago

Thanks @nazli-stripe for hearing me out. Will find a workaround for now 👍

zameschua commented 9 months ago

@nazli-stripe Just a quick update

I managed to change my app to make it stop discovering before any app JS reloads so I don't get the Could not execute discoverReaders because the SDK is busy with another command: discoverReaders error anymore

However I still do run into issues (in development and in production) if the SDK tries to send JS events while the app is reloading.

I'm not sure if there's anything we can do to fix the issue in this repo, but linking to a few related PRs in case anyone encounters the same issue.

https://github.com/facebook/react-native/issues/41188 https://github.com/facebook/react-native/issues/34105