Skip to content

Conversation

@sejas
Copy link
Member

@sejas sejas commented Apr 22, 2025

Related issues

Proposed Changes

  • Avoid using Symlink for current log file.
  • Return the real path to logFile even after rotations.
  • Keep parity between OS.

Testing Instructions

  • Run npm start
  • Go to Import / Export tab
  • Try importing this broken zip file empty.txt.zip
  • Observe the alert and click on Open logs
  • Observe the correct log file opens correctly
  • Test on Mac and Windows

To test if it works after a few rotations I applied this diff and repeated the test instructions a couple of times:

diff --git a/src/logging.ts b/src/logging.ts
index 7cc49e66..d0827ad5 100644
--- a/src/logging.ts
+++ b/src/logging.ts
@@ -109,5 +109,6 @@ export function getLogsFilePath(): string {
 	if ( ! currentLogFile ) {
 		throw new Error( 'Logging system not initialized' );
 	}
+	logStream?.rotate( true );
 	return currentLogFile;
 }
import-site-open-logs-with-real-name.mp4

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

Copy link
Contributor

@gavande1 gavande1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested this and it works as expected. LGTM 👍

@sejas sejas merged commit 7539868 into trunk Apr 23, 2025
8 checks passed
@sejas sejas deleted the update/fix-logs-real-path branch April 23, 2025 08:11
@wojtekn
Copy link
Contributor

wojtekn commented Apr 23, 2025

Thanks for the fix @sejas.

Should we update a Q&A entry to cover for the change?

@sejas
Copy link
Member Author

sejas commented Apr 23, 2025

@wojtekn , good catch! Thanks for the suggestion. I've updated the documentation in English and Spanish.
I considered merging both instruction steps into one, but since each platform uses a different path, I preferred to keep them separate.

Before After
Screenshot 2025-04-23 at 10 34 48 Screenshot 2025-04-23 at 10 42 57

@wojtekn
Copy link
Contributor

wojtekn commented Apr 23, 2025

@sejas looks great, thanks!

bgrgicak pushed a commit that referenced this pull request Apr 23, 2025
* Update getLogsFilePath to return the real path to current logs regardless of the operating system to fix open logs on Windows
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.

4 participants