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

feat: Add logs to ease debugging #844

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: Add logs to ease debugging #844

wants to merge 2 commits into from

Conversation

Ldoppea
Copy link
Member

@Ldoppea Ldoppea commented Jun 22, 2023

This PR adds some logs to ease debugging. Those are basically logs on functions entry but also logs on mount/unmount for CozyAppScreen and HomeView

Ldoppea added 2 commits June 22, 2023 16:22
The ReloadInterceptorWebView component is responsible to intercept the
WebView's reload events and to remount the component instead of just
doing an HTML reload

With current implementation, if the cozy-home app is in an error state
due to invalid cookies, then reloading ReloadInterceptorWebView won't
fix the problem as the same HTML content will be served from the
CozyProxyWebView that is not re-rendered

To fix this, we want to do the reload process on the CozyProxyWebView
instead, so the getIndexHtmlForSlug is called and then cookies would be
refreshed
@Ldoppea Ldoppea requested review from acezard and zatteo as code owners June 22, 2023 14:55
@Crash--
Copy link
Contributor

Crash-- commented Jun 22, 2023

How are we going to access these logs?

@Ldoppea
Copy link
Member Author

Ldoppea commented Jun 22, 2023

They would be visible in the extracted file if we re-use this PR someday: #325

Until then, today the only way to access them is from the dev console, and maybe from Sentry if they are followed by an error (however I'm not sure about the exact rule here).

Example of a Sentry log that contains an exception and show some previous log.debug() calls:
image

@Crash--
Copy link
Contributor

Crash-- commented Jun 22, 2023

Yep but in our case, we don't have any raised error, so the debug log will not be sent. So with this, ATM, it's not useful, no?

What about creating a flag to activate on the cozy that sent these debug logs for this feature?

@Ldoppea
Copy link
Member Author

Ldoppea commented Jun 22, 2023

I'm not sure that sending all debug logs to Sentry would be a good idea (this would mean a lot of new Sentry issues that would be difficult to classify)

However maybe we can log those on a file (only if the flag exists as it may reduce performances) and then add an option to download/share the file from cozy settings?

If we do this we should improve the file logging a bit to make sure that the file won't grow too big

@Crash--
Copy link
Contributor

Crash-- commented Jun 22, 2023

So as for today, this PR is not useful. Right? If yes, should we close it?

Base automatically changed from feat/full_reload_proxy_reloadintereceptor to master June 26, 2023 12:12
@Crash--
Copy link
Contributor

Crash-- commented Jul 10, 2023

ping @Ldoppea

@acezard acezard removed their request for review January 17, 2024 13:45
@Crash--
Copy link
Contributor

Crash-- commented Feb 9, 2024

ping @Ldoppea

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.

2 participants