-
Notifications
You must be signed in to change notification settings - Fork 119
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
[electron] Remove winston as a dependency #3536
Conversation
app/main.development.js
Outdated
); | ||
|
||
process.on("uncaughtException", (err) => { | ||
logger.log("error", "UNCAUGHT EXCEPTION", err); | ||
logger.log("error", "UNCAUGHT EXCEPTION: " + err); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
string template
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work 🥇
Left some minors
app/main_dev/launch.js
Outdated
@@ -700,7 +700,7 @@ export const launchDCRWallet = async ( | |||
e && | |||
e.code && | |||
e.code != "EOF" && | |||
logger.log("error", "tx stream error", e) | |||
logger.log("error", "tx stream error: " + e) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
string template
app/main_dev/launch.js
Outdated
@@ -873,7 +872,7 @@ export const launchDCRLnd = ( | |||
if (!lastDcrdErr || lastDcrdErr === "") { | |||
lastDcrdErr = lastPanicLine(GetDcrdLogs()); | |||
} | |||
logger.log("error", "dcrd closed due to an error: ", lastDcrdErr); | |||
logger.log("error", "dcrd closed due to an error: " + lastDcrdErr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
string template
app/wallet/app.js
Outdated
const logMsg = args.reduce((a) => a + "%s ", "%s "); | ||
const logArgs = [msg, ...args.map(formatArg)]; | ||
const logArgs = args.map(formatArg); | ||
const logMsg = msg + " " + logArgs.join(" "); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
string template
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good and works fine.
I found only one critical issue: when I try to navigate to Logs
view the application quits with this error message:
/home/bgptr/go/src/github.com/decred/decrediton/app/main.js:729
_fs.default.existsSync(walletsDirectory)||_fs.default.mkdirSync(walletsDirectory,{recursive:true});_fs.default.existsSync(mainnetWalletsPath)||_fs.default.mkdirSync(mainnetWalletsPath,{recursive:true});_fs.default.existsSync(testnetWalletsPath)||_fs.default.mkdirSync(testnetWalletsPath,{recursive:true});(0,_paths.checkAndInitWalletCfg)(true);(0,_paths.checkAndInitWalletCfg)(false);logger.log("info","Using config/data from:"+_electron.app.getPath("userData"));logger.log("info",`Versions: Decrediton: ${_electron.app.getVersion()}, `+`Electron: ${process.versions.electron}, `+`Chrome: ${process.versions.chrome}`);process.on("uncaughtException",err=>{logger.log("error",`UNCAUGHT EXCEPTION: ${err}`);throw err;});const installExtensions=async()=>{if(true){const extensions=[_electronDevtoolsInstaller.REACT_DEVELOPER_TOOLS,_electronDevtoolsInstaller.REDUX_DEVTOOLS];const forceDownload=!!process.env.UPGRADE_EXTENSIONS;for(const name of extensions){try{await(0,_electronDevtoolsInstaller.default)(name,forceDownload);}catch(e){console.log("Error installing extension: "+e);}}}};const{ipcMain}=__webpack_require__(/*! electron */"electron");// handleEvent listens on the given channel for ipcRenderer.invoke() calls and
^
TypeError: Cannot read property 'find' of undefined
at IpcMainImpl.<anonymous> (/home/bgptr/go/src/github.com/decred/decrediton/app/main.js:8821:42)
at IpcMainImpl.emit (events.js:315:20)
at Object.<anonymous> (electron/js2c/browser_init.js:161:10351)
at Object.emit (events.js:315:20)
Nice catch! Updated. |
This removes the winston log library as a dependency from the project. This helps reduce the number of dependencies on the main electron layer.
This is done by implementing a very basic logger that replicates the features we were using from winston: writing both to a log file and to the terminal when
debug
is set.Dependencies of the main electron layer before winston is removed:
After removing winston: