use muon.file.writeImportant to write ledger files (#8602) #8605
Conversation
luixxiul
changed the title
app/ledger.js
Outdated
| muon.file.writeImportant(path, JSON.stringify(payload, null, 2), (success) => { | ||
| if (!success) return console.log('write error: ' + path) | ||
|
|
||
| if ((quitP) && (!getSetting(settings.PAYMENTS_ENABLED)) && (getSetting(settings.SHUTDOWN_CLEAR_HISTORY))) { |
There was a problem hiding this comment.
this is kind of weird; so just to confirm... when Brave is exiting (user chose to quit) and payments aren't enabled / history is set to clear, we destroy the file by setting quitP to true and then logging something? I mention it's weird because the deletion only happens after the file is written to (versus just deleting the file).
There was a problem hiding this comment.
yes, it's weird: this deals with a race condition that @NejcZdovc found during testing.
it's done that way because the file may be in the process of being written when we want to delete it. in which case, if we unlink the file, we might just then go ahead and write it again. hence, always delete in this case...
There was a problem hiding this comment.
i'm not sure whether it makes sense. it is certainly counter-intuitive: if you have successfully written the file, and the user is quitting, and the ledger isn't enabled, and history is to be deleted, then get rid of the synopsis file... reminiscent of https://www.youtube.com/watch?v=GoxcyV4DJuY
app/ledger.js
Outdated
| return fs.unlink(path, (err) => { if (err) console.log('unlink error: ' + err.toString()) }) | ||
| } | ||
|
|
||
| if (ledgerInfo._internal.debugP) console.log('\nrenaming ' + path) |
There was a problem hiding this comment.
Is this a stale comment? renaming?
|
no more atomicWriter ... 34d7eb9 ... ok, i'm done, i hope! |
Test Plan
no new tests. existing ledger tests should apply.
Description
git rebase -ito squash commits (if needed).Fixes #8602