Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[VO-258] feat: Allow to send logs my email after clicking on a deep link #1137

Merged
merged 10 commits into from
Feb 26, 2024

Conversation

Ldoppea
Copy link
Member

@Ldoppea Ldoppea commented Jan 19, 2024

In order to allow sending logs whatever if the app is logged or not, or even when the app is stuck on SplashScreen, the easiest way to trigger sending logs is to use deep links

By implementing this, user can now click on cozy://enablelogs or https://links.mycozy.cloud/flagship/enablelogs links to tell the app to start recording logs into a local file

They can then click on cozy://sendlogs or https://links.mycozy.cloud/flagship/sendlogs links to trigger the OS send email intent pre-filled with log files and Cozy's support email

Finally then can click on cozy://disablelogs or https://links.mycozy.cloud/flagship/disablelogs links to tell the app to stop recording logs

This method is not the most user friendly but it has the advantage to be resilient to UI bugs

We may want to find a more user friendly method in the future and keep this one for exceptional cases (or internal ones)


This feature is based on the react-native-file-logger plugin, that does not need to be configured with cozy-minilog as it captures all console calls and redirect them to a file

By default react-native-file-logger will redirect all logs to a file in the app's cache directory

Daily rolling is enabled and the max file size is 5 files of 1MB

We want to decrease the log level to Info so all the app's logs are redirected

Finally ${now} [${level}] is added before all log in the resulting file


TODO:

  • Test on Android (for now Android's build is broken on master)
  • Checks if GPS Memory logs are included
  • Prevent hideSplashscreen() calls to hide the email pane

Improvement ideas:

  • Add another deep link to enable/disable logging to file and disable it by default
  • Add useful data in the email body (current OS, current app version etc)

@Ldoppea Ldoppea marked this pull request as draft January 19, 2024 17:47
@Ldoppea Ldoppea changed the title feat: Allow to send logs my email after clicking on a deep link [VO-258] feat: Allow to send logs my email after clicking on a deep link Jan 22, 2024
@zatteo
Copy link
Contributor

zatteo commented Jan 22, 2024

Add another deep link to enable/disable logging to file and disable it by default

I think it may be needed before releasing.

Checks if GPS Memory logs are included

Currently, GPS Memory messages are logged to a cozy-minilog logger AND to react-native-background-geolocation logger. So it should. In the next weeks, I will try to not send them to Sentry, but I want to keep them in the console for debugging purpose.

@zatteo
Copy link
Contributor

zatteo commented Jan 22, 2024

Can't wait to see it in production. Will be very useful.

@Ldoppea
Copy link
Member Author

Ldoppea commented Feb 12, 2024

I added a new commit to this PR: f121a8b

This ones makes the feature "opt-in" and the user now needs to click a UL to enable login into files.

Also I found a bug on iOS when the app is connected. If for some reason the splashscreen is hidden while the email pane is opened, then the splashscreen plugin will hide the email pane instead (because it blindly hides the topmost view)

I'll have to find a workaround for this, either by showing an email_splashscreen while the email pane is opened, or by finding a way to fix the bootsplash library so it always hide the correct view. I'm not sure yet on to do these

In theory the email_splashscreen option would work great, but I have to find how to wait for the email pane to be closed. For now the method directly resolves its promise when called.

The sendLogFilesByEmail method shows a MFMAilComposeViewController and then resolves
But this view's callback is the didFinishWithResult method which has no access to the promise. I'll have to find a way to keep the promise pointer between the two methods

@Ldoppea Ldoppea force-pushed the feat/send_logs branch 2 times, most recently from bbd0686 to 94c371e Compare February 16, 2024 19:14
@Ldoppea Ldoppea marked this pull request as ready for review February 16, 2024 19:14
@Ldoppea
Copy link
Member Author

Ldoppea commented Feb 16, 2024

This PR is ready for review, I fixed previous issue by patching react-native-file-logger package in 9157df2

This package is needed by the following commits to ease debugging by
allowing the user to send their app's log by email
By default `react-native-file-logger` will redirect all logs to a file
in the app's cache directory

Daily rolling is enabled and the max file size is 5 files (5 days) of
1MB

We want to decrease the log level to `Info` so all the app's logs are
redirected

Finally `${now} [${level}]` is added before all log in the resulting
file
In order to allow sending logs whatever if the app is logged or not, or
even when the app is stuck on SplashScreen, the easiest way to trigger
sending logs is to use deep links

By implementing this, user can now click on `cozy://sendlogs` or
`https://links.mycozy.cloud/flagship/sendlogs` links to trigger the OS
send email intent pre-filled with log files and Cozy's  support email

This method is not the most user friendly but it has the advantage to
be resilient to UI bugs

We may want to find a more user friendly method in the future and keep
this one for exceptional cases (or internal ones)
In previous commits we implemented a File logger that would intercept
all console logs and write them in a file

We want this feature to be opt-in in order to respect user's privacy

The user can now click on `cozy://enableLogs` or
`https://links.mycozy.cloud/flagship/enableLogs` links to activate the
File logger interception

They can also click on `cozy://disableLogs` or
`https://links.mycozy.cloud/flagship/disableLogs` links to deactivate
the File logger interception. This action also delete existing log
files

Also when the user clicks on the `sendLogs` link, an error message is
now displayed if the logs are disabled
In the following commit we will need to wait for the iOS email to be
sent

Current `react-native-file-logger` implementation calls the iOS Mail
composer and then directly calls the `resolve()` method

Instead we want to wait for the email to be sent and then call
`resolve()` from the `didFinishWithResult` method

To make this possible we need to store a pointer to the `resolve` and
the `reject` methods. This will allow us to call those pointers from
the `didFinishWithResult` method
On iOS, the `FileLogger.sendLogFilesByEmail()` method shows the
`MFMailComposeViewController` on top of the app UI

Because we call this when receiving an Universal Link clicked outside
of the app, this means that the Mail composer will be called when the
app gets the focus back. The result is that the Mail composer may
temporarily be displayed on top of the SplashScreen

This is problematic due to the way `react-native-bootsplash` works on
iOS

When calling `hideSplashScreen` this module will try to hide the
topmost UIViewController. This means that if the Mail composer is
displayed on top of the SplashScreen, then `hideSplashScreen` will hide
the Mail composer instead of the SplashScreen

To prevent this we want to be sure that the SplashScreen won't be
hidden until the Mail composer is closed. This can be done by
displaying a named SplashScreen before calling
`FileLogger.sendLogFilesByEmail()`, then the SplashScreen will be kept
on screen even if `hideSplashScreen` is called for the lock screen or
the global SplashScreen

In the future we may want to also fix the `react-native-bootsplash`
behaviour

Note that this problem does not occur on Android because an external
intent is called and also because the `react-native-bootsplash`
implementation is more robust on this OS
@Ldoppea Ldoppea merged commit da270cf into master Feb 26, 2024
1 check passed
@Ldoppea Ldoppea deleted the feat/send_logs branch February 26, 2024 11:43
Ldoppea added a commit that referenced this pull request Feb 29, 2024
In #1137 we implemented a new File logger mechanism

Unfortunately this has a side effect on
react-native-background-geolocation which has a logger mechanism based
on the same library and fails to work if we enable our new File logger

A solution is investigated in BeTomorrow/react-native-file-logger#64
but until then we want to disable the feature so we can continue
publishing the app

Related PR: #1137
Related Issue: BeTomorrow/react-native-file-logger#64
@Ldoppea Ldoppea mentioned this pull request Feb 29, 2024
4 tasks
Ldoppea added a commit that referenced this pull request Feb 29, 2024
In #1137 we implemented a new File logger mechanism

Unfortunately this has a side effect on
react-native-background-geolocation which has a logger mechanism based
on the same library and fails to work if we enable our new File logger

A solution is investigated in BeTomorrow/react-native-file-logger#64
but until then we want to disable the feature so we can continue
publishing the app

Related PR: #1137
Related Issue: BeTomorrow/react-native-file-logger#64
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants