Need to catch Peast exceptions

THANK YOU for this. I was running down a big rabbit hole when JS aggregation was on trying to find out what was causing my errors and this saved me.

Good idea - can you open an MR for the branch?

To test this - do we need js with a syntax exception that Peast will definitely throw an exception for?

I opened #3382123: Update Peast to 1.15.4 to update Peast too.

This seems to also resolve the case where you mistakenly don't mark a library as minified: https://www.drupal.org/project/drupal/issues/3371010

Rebased and Fixed issues with tests failing. Needs Review

Couple of comments on the MR but overall looking good.

Went with the default system logger as there isn't any other instances of asset logging to really warrant it's own channel.

Looks great. Will mean that actually broken JavaScript won't cause any more problems than it would have done in 10.x, and if Peast has its own parsing errors on unusual/new syntax that we still get a perfectly usable aggregate, just without that particular file minimized.

Good point yes we do.

Here's a test only patch that should fail.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Rebased/retargeted onto 11.x

Looks good!

Here's the interdiff of the test coverage I added:

diff --git a/core/tests/Drupal/Tests/Core/Asset/JsOptimizerUnitTest.php b/core/tests/Drupal/Tests/Core/Asset/JsOptimizerUnitTest.php
index 15f21a22cf0062f0e16a860257e069ab8c6b0e10..3c4623ab228fdb1c3df9573a041dce6b8a046296 100644
--- a/core/tests/Drupal/Tests/Core/Asset/JsOptimizerUnitTest.php
+++ b/core/tests/Drupal/Tests/Core/Asset/JsOptimizerUnitTest.php
@@ -119,6 +119,16 @@ public function providerTestOptimize() {
         ],
         file_get_contents($path . 'to_be_minified.js.optimized.js'),
       ],
+      4 => [
+        [
+          'type' => 'file',
+          'preprocess' => TRUE,
+          'data' => $path . 'syntax_error.js',
+        ],
+        // When there is a syntax error, the 'optimized' contents are the
+        // contents of the original file.
+        file_get_contents($path . 'syntax_error.js'),
+      ],
     ];
   }
 
diff --git a/core/tests/Drupal/Tests/Core/Asset/js_test_files/syntax_error.js b/core/tests/Drupal/Tests/Core/Asset/js_test_files/syntax_error.js
new file mode 100644
index 0000000000000000000000000000000000000000..3794c41c8451c98cfcbbaba6b6839263f3aeebbc
--- /dev/null
+++ b/core/tests/Drupal/Tests/Core/Asset/js_test_files/syntax_error.js
@@ -0,0 +1,13 @@
+/**
+ * Some comments.
+ */
+
+(function foo() {
+  // Missing closing parenthesis.
+  if (true {
+    'print 1';
+  }
+  else {
+    'print 2';
+  }
+})

Given that was the only code I touched here, I'm going to go ahead and commit it. Committed/pushed to 11.x and cherry-picked to 10.1.x, thanks!