feat: Debug console view #36337

TMisiukiewicz · 2024-02-12T14:11:13Z

Details

Implementing the Debug console view, which displays logs from Logger in real time, with a possibility of saving it to the .txt file or sharing it as an attachment in a report.

Fixed Issues

$ #34082
PROPOSAL:

Tests

Go to Profile -> About -> Troubleshoot
Verify "Client side logging" is disabled
Enable "Client side logging"
Verify "View debug console" option appears
Press "View debug console"
Verify Debug console screen is open and logs are displayed in a console in real-time
Type 5+5 in the input below the console and press "Execute"
Verify 10 was displayed in a console
Press "Save log"
Verify file is downloaded to a device
Press "Share log"
Verify you are navigated to the search page
Press on any report
Verify you are moved to the report and logs are sent as an attachment

Verify that no errors appear in the JS console

Offline tests

n/a

QA Steps

Go to Profile -> About -> Troubleshoot
Verify "Client side logging" is disabled
Enable "Client side logging"
Verify "View debug console" option appears
Press "View debug console"
Verify Debug console screen is open and logs are displayed in a console in real-time
Type 5+5 in the input below the console and press "Execute"
Verify 10 was displayed in a console
Press "Save log"
Verify file is downloaded to a device
Press "Share log"
Verify you are navigated to the search page
Press on any report
Verify you are moved to the report and logs are sent as an attachment

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

ANDROID.mov

Android: mWeb Chrome

ANDROID.WEB.mov

iOS: Native

IOS.mov

iOS: mWeb Safari

IOS.WEB.mov

MacOS: Chrome / Safari

WEB.mov

MacOS: Desktop

DESKTOP.mov

This reverts commit 88d2e8b.

melvin-bot · 2024-02-14T12:32:24Z

@fedirjh Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

fedirjh · 2024-02-14T13:21:21Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android: Native
- Android: mWeb Chrome
- iOS: Native
- iOS: mWeb Safari
- MacOS: Chrome / Safari
- MacOS: Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If the PR modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label so the design team can review the changes.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native

CleanShot.2024-02-14.at.15.23.50.mp4

Android: mWeb Chrome

CleanShot.2024-02-14.at.15.29.59.mp4

iOS: Native

CleanShot.2024-02-14.at.13.58.13.mp4

iOS: mWeb Safari

CleanShot.2024-02-14.at.14.18.03.mp4

MacOS: Chrome / Safari

CleanShot.2024-02-14.at.13.50.40.mp4

MacOS: Desktop

CleanShot.2024-02-14.at.15.36.33.mp4

fedirjh

Looks good. Left some comments about the code.

fedirjh · 2024-02-14T13:41:31Z

src/components/Button/index.tsx

@@ -320,7 +320,7 @@ function Button(
                    shouldRemoveRightBorderRadius ? styles.noRightBorderRadius : undefined,
                    shouldRemoveLeftBorderRadius ? styles.noLeftBorderRadius : undefined,
                    // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
-                    'text' in rest && (rest?.icon || rest?.shouldShowRightIcon) ? styles.alignItemsStretch : undefined,
+                    'text' in rest && rest?.shouldShowRightIcon ? styles.alignItemsStretch : undefined,


Any reason for this change?

Confirmation for that change was here

Looks good, this code was added in #30417 to fix this bug #30400. We are safe here:

fedirjh · 2024-02-14T13:43:42Z

src/libs/localFileCreate/index.native.ts

+ * @param textContent content of the file
+ * @returns path, filename and size of the newly created file
+ */
+const localFileCreate = (fileName: string, textContent: string) => {


Let's create a new types.ts file and add the type for localFileCreate

type LocalFileCreate = (fileName: string, textContent: string) => Promise<{path: string, newFileName: string, size: number}>

fedirjh · 2024-02-14T13:47:17Z

src/libs/localFileDownload/index.ios.ts

@@ -1,6 +1,6 @@
 import {Share} from 'react-native';
 import RNFetchBlob from 'react-native-blob-util';
-import * as FileUtils from '@libs/fileDownload/FileUtils';
+import localFileCreate from '@libs/localFileCreate/index.native';


Suggested change

import localFileCreate from '@libs/localFileCreate/index.native';

import localFileCreate from '@libs/localFileCreate';

fedirjh · 2024-02-14T13:54:18Z

src/libs/localFileDownload/index.ts

+    localFileCreate(fileName, textContent).then(({path}) => {
+        const link = document.createElement('a');
+        link.download = FileUtils.appendTimeToFileName(`${fileName}.txt`);
+        link.href = path;
+        link.click();
+    });


Suggested change

localFileCreate(fileName, textContent).then(({path}) => {

const link = document.createElement('a');

link.download = FileUtils.appendTimeToFileName(`${fileName}.txt`);

link.href = path;

link.click();

});

localFileCreate(`${fileName}.txt`, textContent).then(({path, newFileName}) => {

const link = document.createElement('a');

link.download = newFileName;

link.href = path;

link.click();

});

localFileCreate should append Time to FileName

fedirjh · 2024-02-14T13:56:54Z

src/pages/settings/AboutPage/ConsolePage.tsx

+ * @param logs Logs captured on the current device
+ * @returns CapturedLogs with parsed messages
+ */
+const parseStingifiedMessages = (logs: Log[]) => {


Suggested change

const parseStingifiedMessages = (logs: Log[]) => {

const parseStringifyMessages = (logs: Log[]) => {

little typo

fedirjh · 2024-02-14T13:57:10Z

src/pages/settings/AboutPage/ConsolePage.tsx

+    useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ENTER, executeArbitraryCode);
+
+    const saveLogs = () => {
+        const logsWithParsedMessages = parseStingifiedMessages(logsList);


Suggested change

const logsWithParsedMessages = parseStingifiedMessages(logsList);

const logsWithParsedMessages = parseStringifyMessages(logsList);

fedirjh · 2024-02-14T13:57:47Z

src/pages/settings/AboutPage/ConsolePage.tsx

+
+    const shareLogs = () => {
+        setIsGeneratingLogsFile(true);
+        const logsWithParsedMessages = parseStingifiedMessages(logsList);


Suggested change

const logsWithParsedMessages = parseStingifiedMessages(logsList);

const logsWithParsedMessages = parseStringifyMessages(logsList);

fedirjh · 2024-02-14T14:03:06Z

src/pages/settings/AboutPage/ConsolePage.tsx

+        }
+
+        setLogs((prevLogs) => ({...prevLogs, ...capturedLogs}));
+        // eslint-disable-next-line react-hooks/exhaustive-deps


Could you please add a comment explaining why this is needed? Just to make sure it won't be missed and removed in the future

I think it was some kind of leftover that I missed, I think it is safe to remove it and pass shouldStoreLogs to dependency array as well

fedirjh · 2024-02-14T14:13:43Z

src/pages/settings/AboutPage/ShareLogPage.tsx

+type ShareLogPageProps = {
+    route: {
+        params: {
+            source: string;
+        };
+    };
+};


Suggested change

type ShareLogPageProps = {

route: {

params: {

source: string;

};

};

};

type ShareLogPageProps = StackScreenProps<SettingsNavigatorParamList, typeof SCREENS.SETTINGS.SHARE_LOG>;

fedirjh

Looks good to me.

fedirjh · 2024-02-14T17:34:55Z

Verify "View debug console" option appears

@TMisiukiewicz Just a little question about this behavior: Why the View debug console option appears only if we enable client side debugging? I think it will be helpful if we can still use the debug console without the client side debugging?

techievivek

Looks good to me a few NABs that we can tackle later as well.

techievivek · 2024-02-14T17:18:10Z

src/libs/Console/index.ts

+    originalConsoleLog.apply(console, args);
+};
+
+const charsToSanitize = /[\u2018\u2019\u201C\u201D\u201E\u2026]/g;


It would be nice to add a comment explaining a few of these chars and why we need to sanitize.

techievivek · 2024-02-14T17:21:25Z

src/libs/Console/index.ts

+
+const charsToSanitize = /[\u2018\u2019\u201C\u201D\u201E\u2026]/g;
+
+const charMap: Record<string, string> = {


Maybe move this to before charsToSanitize variable so we can map out unicode to string literals.

techievivek · 2024-02-14T17:39:16Z

src/pages/settings/AboutPage/ConsolePage.tsx

+
+        // Generate a file with the logs and pass its path to the list of reports to share it with
+        localFileCreate('logs', JSON.stringify(logsWithParsedMessages, null, 2)).then(({path, size}) => {
+            setIsGeneratingLogsFile(false);


NAB: Use finally for resetting the state?

techievivek · 2024-02-14T17:41:15Z

src/pages/settings/AboutPage/ConsolePage.tsx

+
+    const executeArbitraryCode = () => {
+        const sanitizedInput = sanitizeConsoleInput(input);
+


NAB: Extra line, we can avoid the new lines if the variable is used immediately in the next line.

techievivek · 2024-02-14T17:41:31Z

src/pages/settings/AboutPage/ConsolePage.tsx

+
+    const saveLogs = () => {
+        const logsWithParsedMessages = parseStringifyMessages(logsList);
+


NAB: new line.

techievivek · 2024-02-14T17:45:31Z

@TMisiukiewicz Just a little question about this behavior: Why the View debug console option appears only if we enable client side debugging? I think it will be helpful if we can still use the debug console without the client side debugging?

Yeah, I agree. We can make the debug console accessible to users even if when they haven't turned on client side debugging.

techievivek · 2024-02-14T18:02:04Z

None of the above comments are concerning,, so I am going ahead with the merge.

We have one discussion about having the debug console feature independently available https://expensify.slack.com/archives/C01GTK53T8Q/p1707932836441509?thread_ts=1703982689.203629&cid=C01GTK53T8Q, which we can tackle after a consensus.

OSBotify · 2024-02-14T18:08:34Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2024-02-15T05:37:16Z

🚀 Deployed to staging by https://github.com/techievivek in version: 1.4.42-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2024-02-19T23:35:24Z

🚀 Deployed to production by https://github.com/chiragsalian in version: 1.4.42-5 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

abzokhattab · 2024-03-16T00:12:57Z

src/libs/OptionsListUtils.ts

+    return getOptions(reports, personalDetails, {
+        betas,
+        searchInputValue: searchValue.trim(),
+        includeRecentReports: true,
+        includeMultipleParticipantReports: true,
+        sortByReportTypeInSearch: true,
+        includePersonalDetails: true,
+        forcePolicyNamePreview: true,
+        includeOwnedWorkspaceChats: true,
+    });


Hello @TMisiukiewicz @fedirjh @techievivek

coming from this issue in which we want to modify the getOptions params to include also the direct DM as one of the returned result, and I was suggesting here to include also the threads.

So we are wondering if you have already discussed the possibility of enabling threads in the returned list.

cc @mkhutornyi @CortneyOfstad

There might be deleted threads in the search results as well, and hence they might have disabled the threads option 🤔

Is this the case why threads were disabled?

just a friendly reminder @TMisiukiewicz @fedirjh @techievivek

cc @abzokhattab I don't know if there have been any discussions about allowing or preventing the sharing of logs within threads.

hungvu193 · 2024-03-18T08:17:09Z

src/pages/settings/AboutPage/ShareLogPage.tsx

+type ShareLogPageProps = StackScreenProps<SettingsNavigatorParamList, typeof SCREENS.SETTINGS.SHARE_LOG>;
+
+function ShareLogPage({route}: ShareLogPageProps) {
+    return <ShareLogList logSource={route.params.source} />;


While reviewing another PR, I found out that there will be crash if we open ShareLogPage without source query param. So I think we should add optional chaining here or wrap ShareLogPage with FullPageNotFound view where we don't have source query param.

Screen.Recording.2024-03-18.at.15.16.31.mov

getusha · 2024-04-10T11:29:49Z

src/pages/settings/AboutPage/ConsolePage.tsx

+                    data={logsList}
+                    renderItem={renderItem}
+                    contentContainerStyle={styles.p5}
+                    inverted


We should have used the existing InvertedFlatList, which has maintainVisibleContentPosition set, to avoid auto-scrolling on new data. This caused a regression.

#38155

TMisiukiewicz added 30 commits February 2, 2024 14:45

initial console to execute js code

8b584bf

cleanup console code

d040a4b

update console

b4aee78

add save logs feature

05919f1

parse object messages

d70662a

add captured logs to console

f2a3cf8

create console page

008418c

move away from console component to console page

35396bc

use onyx to collect logs

ab22eaf

use flashlist for rendering console logs

d21fb5e

update console padding

0044ef5

use localize in console page

1d17879

display logs only if they are available

88d2e8b

Revert "display logs only if they are available"

95bfd46

This reverts commit 88d2e8b.

render flashlist when logs are available

3eced55

update buttons

0d903a0

use flex on console view

6b5cc8f

add comments to not described functions

b106c44

add share log page

820dc4e

connect logs with search for sharing log

abcf6d7

share logs in a chat on mobile

03ee43b

add loader for sharing

0e1ddaf

store logs only after user enters the console page

33fef18

send logs on Android

e6c6299

cross-platform logs sharing

4227eaf

code cleanup

25a3c26

migrate console view to flatlist

6a24e8a

fix typings

1ff340d

update behavior for client side logging

76b30b1

update log time

f250613

TMisiukiewicz added 4 commits February 14, 2024 08:44

Merge branch 'feat/troubleshooting-section' into feat/console

a0574d5

add alert when file exceeds the limit

0bdbcf4

Merge branch 'main' into feat/console

2621222

remove unnecessary console debug

bbbdd16

TMisiukiewicz marked this pull request as ready for review February 14, 2024 12:32

TMisiukiewicz requested a review from a team as a code owner February 14, 2024 12:32

melvin-bot bot requested review from fedirjh and removed request for a team February 14, 2024 12:32

fedirjh reviewed Feb 14, 2024

View reviewed changes

TMisiukiewicz added 2 commits February 14, 2024 15:48

code review updates

025a9a0

update share log page nav props

96b08ec

fedirjh approved these changes Feb 14, 2024

View reviewed changes

melvin-bot bot requested a review from techievivek February 14, 2024 15:46

techievivek approved these changes Feb 14, 2024

View reviewed changes

techievivek merged commit 51c0ecc into Expensify:main Feb 14, 2024
18 checks passed

github-actions bot mentioned this pull request Feb 15, 2024

Deploy Checklist: New Expensify 2024-02-15 #36558

Closed

73 tasks

TMisiukiewicz mentioned this pull request Feb 16, 2024

[NO QA] fix: collecting logs on dev #36659

Merged

50 tasks

trjExpensify mentioned this pull request Feb 23, 2024

[HOLD for payment 2024-03-07] Desktop - App becomes unusable afterclear cache and restart #37116

Closed

6 tasks

abzokhattab reviewed Mar 16, 2024

View reviewed changes

hungvu193 reviewed Mar 18, 2024

View reviewed changes

getusha reviewed Apr 10, 2024

View reviewed changes

getusha mentioned this pull request Apr 10, 2024

[HOLD for payment 2024-04-09] [$500] Scroll position jumps to new data each time new data comes in debug console #38155

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Debug console view #36337

feat: Debug console view #36337

TMisiukiewicz commented Feb 12, 2024 •

edited

Loading

melvin-bot bot commented Feb 14, 2024

fedirjh commented Feb 14, 2024 •

edited

Loading

fedirjh left a comment

fedirjh Feb 14, 2024

TMisiukiewicz Feb 14, 2024

fedirjh Feb 14, 2024

fedirjh Feb 14, 2024

fedirjh Feb 14, 2024

fedirjh Feb 14, 2024 •

edited

Loading

fedirjh Feb 14, 2024

fedirjh Feb 14, 2024

fedirjh Feb 14, 2024

fedirjh Feb 14, 2024

TMisiukiewicz Feb 14, 2024

fedirjh Feb 14, 2024

fedirjh left a comment

fedirjh commented Feb 14, 2024

techievivek left a comment

techievivek Feb 14, 2024

techievivek Feb 14, 2024

techievivek Feb 14, 2024

techievivek Feb 14, 2024

techievivek Feb 14, 2024

techievivek commented Feb 14, 2024

techievivek commented Feb 14, 2024

OSBotify commented Feb 14, 2024

OSBotify commented Feb 15, 2024

OSBotify commented Feb 19, 2024

abzokhattab Mar 16, 2024

allgandalf Mar 16, 2024

abzokhattab Mar 20, 2024

fedirjh Mar 20, 2024

hungvu193 Mar 18, 2024

getusha Apr 10, 2024

	import localFileCreate from '@libs/localFileCreate/index.native';
	import localFileCreate from '@libs/localFileCreate';

	const parseStingifiedMessages = (logs: Log[]) => {
	const parseStringifyMessages = (logs: Log[]) => {

	const logsWithParsedMessages = parseStingifiedMessages(logsList);
	const logsWithParsedMessages = parseStringifyMessages(logsList);


		const charsToSanitize = /[\u2018\u2019\u201C\u201D\u201E\u2026]/g;

		const charMap: Record<string, string> = {


		const executeArbitraryCode = () => {
		const sanitizedInput = sanitizeConsoleInput(input);


		const saveLogs = () => {
		const logsWithParsedMessages = parseStringifyMessages(logsList);

feat: Debug console view #36337

feat: Debug console view #36337

Conversation

TMisiukiewicz commented Feb 12, 2024 • edited Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

melvin-bot bot commented Feb 14, 2024

fedirjh commented Feb 14, 2024 • edited Loading

Reviewer Checklist

Screenshots/Videos

fedirjh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fedirjh Feb 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fedirjh left a comment

Choose a reason for hiding this comment

fedirjh commented Feb 14, 2024

techievivek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

techievivek commented Feb 14, 2024

techievivek commented Feb 14, 2024

OSBotify commented Feb 14, 2024

OSBotify commented Feb 15, 2024

OSBotify commented Feb 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TMisiukiewicz commented Feb 12, 2024 •

edited

Loading

fedirjh commented Feb 14, 2024 •

edited

Loading

fedirjh Feb 14, 2024 •

edited

Loading