Better handle intermittent JSON parsing error

[bsclifton]

Fixes #6189
Fixes #7207

• Workaround was put in place for Windows use-case in #2787
• This appears to work most of the time on Linux 👍(always works other platforms)
• There are cases where appending this breaks the parsing (ex: last line looks like {"phase":2,"source":{"id":73,"type":8}]})
• In those cases, fall back is to what the string was before ending with }]}

Root cause not known. Not sure if Node.js fs.readFileSync is waiting for the file to finish writing?

Submitter Checklist:

• Submitted a ticket for my issue if one did not already exist.
• Used Github auto-closing keywords in the commit message.
• Added/updated tests for this change (for new code or code which already has tests).
• Verified that these changes build without errors on
  • Android
  • iOS
  • Linux
  • macOS
  • Windows
• Verified that these changes pass automated tests (unit, browser, security tests) on
  • iOS
  • Linux
  • macOS
  • Windows
• Ran git rebase master (if needed).
• Ran git rebase -i to squash commits (if needed).
• Tagged reviewers and labelled the pull request as needed.
• Requested a security/privacy review as needed.
• Public documentation has been updated as necessary. For instance:
  • https://github.com/brave/brave-browser/wiki/Deviations-from-Chromium-(features-we-disable-or-remove)
  • https://github.com/brave/brave-browser/wiki/Proxy-redirected-URLs
  • https://github.com/brave/brave-browser/wiki/Fingerprinting-Protection-Mode
  • https://github.com/brave/brave-browser/wiki/Brave%E2%80%99s-Use-of-Referral-Codes
  • https://github.com/brave/brave-browser/wiki/Custom-Headers
  • https://github.com/brave/brave-browser/wiki/Web-compatibility-issues-with-tracking-protection

Test Plan:

1. Build on Linux (Release)
2. Run npm run network-audit -- --output_path=src/out/Release/brave
3. Try a few times; see if you encounter the log statement
4. Check CI for this PR; verify Linux run (and other platforms) worked great

Reviewer Checklist:

• New files have MPL-2.0 license header.
• Request a security/privacy review as needed.
• Adequate test coverage exists to prevent regressions.

After-merge Checklist:

• The associated issue milestone is set to the smallest version that the
  changes has landed on.
• All relevant documentation has been updated.

[bsclifton]

Issue ran into again (nice) and here's how the fix handled it:

01:38:29  Error parsing JSON SyntaxError: Unexpected token ] in JSON at position 49499689
01:38:29      at JSON.parse (<anonymous>)
01:38:29      at start (/home/ubuntu/jenkins/workspace/brave-browser-build-pr_PR-7182/lib/start.js:147:25)
01:38:29      at Command.listener (/home/ubuntu/jenkins/workspace/brave-browser-build-pr_PR-7182/node_modules/commander/index.js:315:8)
01:38:29      at Command.emit (events.js:210:5)
01:38:29      at Command.parseArgs (/home/ubuntu/jenkins/workspace/brave-browser-build-pr_PR-7182/node_modules/commander/index.js:654:12)
01:38:29      at Command.parse (/home/ubuntu/jenkins/workspace/brave-browser-build-pr_PR-7182/node_modules/commander/index.js:474:21)
01:38:29      at Object.<anonymous> (/home/ubuntu/jenkins/workspace/brave-browser-build-pr_PR-7182/scripts/commands.js:177:4)
01:38:29      at Module._compile (internal/modules/cjs/loader.js:956:30)
01:38:29      at Object.Module._extensions..js (internal/modules/cjs/loader.js:973:10)
01:38:29      at Module.load (internal/modules/cjs/loader.js:812:32) Trying the original non-transformed text
01:38:29  undefined:16305
01:38:29  {"phase":2,"source":{"id":99,"type":8},"time":"12105830","type":34}
01:38:29                                                                     
01:38:29  
01:38:29  SyntaxError: Unexpected end of JSON input
01:38:29      at JSON.parse (<anonymous>)
01:38:29      at start (/home/ubuntu/jenkins/workspace/brave-browser-build-pr_PR-7182/lib/start.js:152:27)
01:38:29      at Command.listener (/home/ubuntu/jenkins/workspace/brave-browser-build-pr_PR-7182/node_modules/commander/index.js:315:8)
01:38:29      at Command.emit (events.js:210:5)
01:38:29      at Command.parseArgs (/home/ubuntu/jenkins/workspace/brave-browser-build-pr_PR-7182/node_modules/commander/index.js:654:12)
01:38:29      at Command.parse (/home/ubuntu/jenkins/workspace/brave-browser-build-pr_PR-7182/node_modules/commander/index.js:474:21)
01:38:29      at Object.<anonymous> (/home/ubuntu/jenkins/workspace/brave-browser-build-pr_PR-7182/scripts/commands.js:177:4)
01:38:29      at Module._compile (internal/modules/cjs/loader.js:956:30)
01:38:29      at Object.Module._extensions..js (internal/modules/cjs/loader.js:973:10)
01:38:29      at Module.load (internal/modules/cjs/loader.js:812:32)

Gonna download the log file and look at a different fix; looks like this log is being cut off and needs proper formation before it parses as JSON

[bsclifton]

Passed on all except Android; going to re-run Android + Linux. Might run Linux a few times, to see if it hits the failed parse condition 👍

[bsclifton]

CI passed great on Android this time; Linux also worked without issue. Gonna try one more Linux run, then I think this is ready for consideration

[bsclifton]

Perfect! The last build ran into the issue and this work-around worked great 😄 Here's a snippet from the logs:
https://ci.brave.com/job/brave-browser-build-pr/job/PR-7182/8/execution/node/376/log/

16:29:09  Waiting for 10 seconds before reading "/home/ubuntu/jenkins/workspace/brave-browser-build-pr_PR-7182/network_log.json"
16:29:09  Error parsing JSON SyntaxError: Unexpected token ] in JSON at position 49522853
16:29:09      at JSON.parse (<anonymous>)
16:29:09      at start (/home/ubuntu/jenkins/workspace/brave-browser-build-pr_PR-7182/lib/start.js:154:25)
16:29:09      at Command.listener (/home/ubuntu/jenkins/workspace/brave-browser-build-pr_PR-7182/node_modules/commander/index.js:315:8)
16:29:09      at Command.emit (events.js:210:5)
16:29:09      at Command.parseArgs (/home/ubuntu/jenkins/workspace/brave-browser-build-pr_PR-7182/node_modules/commander/index.js:654:12)
16:29:09      at Command.parse (/home/ubuntu/jenkins/workspace/brave-browser-build-pr_PR-7182/node_modules/commander/index.js:474:21)
16:29:09      at Object.<anonymous> (/home/ubuntu/jenkins/workspace/brave-browser-build-pr_PR-7182/scripts/commands.js:177:4)
16:29:09      at Module._compile (internal/modules/cjs/loader.js:956:30)
16:29:09      at Object.Module._extensions..js (internal/modules/cjs/loader.js:973:10)
16:29:09      at Module.load (internal/modules/cjs/loader.js:812:32)
16:29:09  Trying a second work-around
16:29:09  network audit passed.

[bsclifton]

@bridiver updated! Ready for re-review

[bsclifton]

Per #7207 (comment), I added a commit removing the hacks and then using SIGTERM instead. CI will run again, let's see how it goes

[bsclifton]

Looks like even with latest revision (which does SIGTERM, not SIGKILL) Windows still doesn't have proper endings in the JSON (ex: it ends the last line with , and is missing the ]})

More discussion about that in #2787 (comment)

Last possible fix I can try would be to only apply the }]} replacement hack if the platform is Windows. Seems the build did finish as expected on all platforms without having to be killed

[bsclifton]

Looks like this worked! 😄

Links to the output for network-audit on each platform:

• Linux
• macOS
• Windows

[bsclifton]

@bridiver @diracdeltas this is ready for re-review 😄

SIGKILL was originally put in place with #4036 because PR builder would hang during the old npm run test-security step (eventually, the build would ABORT from reaching the timeout). Sometime after that, the test-security step was broken into two parts (audit_deps and network-audit). In this PR, I removed SIGKILL in favor of SIGTERM and PR builder DID finish as expected on all platforms.

The Windows only hack from #2787 is preserved, but ONLY on Windows (check done via process.platform). macOS and Linux process the logs as-is and did NOT encounter a JSON parse problem. I went into the Jenkins workspace for the macOS instances, downloaded the network-audit.log, and verified they have the proper }]} ending

Given the above, I believe that this PR also solves #7207

[diracdeltas]

this is a way cleaner solution; nice!

[bsclifton]

Created #7281 to follow up on idea @bridiver mentioned here: #7207 (comment)

[bsclifton]

Looking at the PR builder run, noticed the following:

• the Linux job failed ☹️ https://ci.brave.com/job/brave-browser-build-pr/job/PR-7182/13/execution/node/493/log/
• Windows hung during init (not related to this), but seeing that pass would be important https://ci.brave.com/job/brave-browser-build-pr/job/PR-7182/13/execution/node/403/log/

Since a failure was seen even with this PR, I don't think PR this properly solves the issue. The work-around I had before does "fix" the issue, but doesn't ensure integrity of the logs

Going to close this and we'll have to revisit at another time