From Drupal.org's logs:

15:13:08 WD l10n_drupal: Parsing extracted da_vinci-8.x-1.0-alpha1.tar.gz for    [notice]
15:13:08 strings.
15:13:25 WD potx: YAML parsing error on file                                     [notice]
15:13:25 /tmp/l10n_drupal_drupalorg_D0inB3_dir/da_vinci/node_modules/bower/node_modules/github/seed.yml:
15:13:25 Unable to parse at line 2 (near "  name: node-github").
15:14:17 WD potx: YAML parsing error on file                                     [notice]
15:14:17 /tmp/l10n_drupal_drupalorg_D0inB3_dir/da_vinci/node_modules/grunt-sass/node_modules/node-sass/src/libsass/appveyor.yml:
15:14:17 Unable to parse at line 5 (near "  Build: static").
15:14:17 WD cron: Twig_Error_Syntax: Unexpected character "&" in                  [error]
15:14:17 "da_vinci/templates/comments/comment-wrapper.html.twig" at line 38 in
15:14:17 Twig_Lexer->lexExpression() (line 284 of
15:14:17 /var/www/localize.drupal.org/htdocs/sites/all/modules/potx/vendor/Twig/Lexer.php).
15:14:18 WD cron: Cron run completed.                                            [notice]
15:14:18 Cron run successful.                                                   [success]
15:14:18 The first parameter to t() should be a literal string. There should      [error]
15:14:18 be no variables, concatenation, constants or other non-literal
15:14:18 strings there. At t('Name for tab '.$i) in
15:14:18 da_vinci/plugins/styles/region_tabs/region_tabs.inc on line 52. Read
15:14:18 more at http://drupal.org/node/322732
15:14:18 Command dispatch complete                                               [notice]

This leaves temporary files not cleaned up. Twig and YAML parse errors should be caught, logged appropriately, and cleanup completed.

CommentFileSizeAuthor
#8 2613368-8.patch4.59 KBherom
#8 2613368-8-broken-yaml.test-only.patch286 bytesherom
#6 2613368-6.patch4.55 KBherom
#6 2613368-6-broken-twig.test-only.patch218 bytesherom
#6 2613368-6-broken-yaml.test-only.patch213 bytesherom
#4 2613368-4.patch4.13 KBherom
#4 interdiff-2613368-2-4.txt1.34 KBherom
#2 2613368-2.patch3.67 KBherom

Comments

drumm created an issue. See original summary.

herom’s picture

herom commented
Status: Active » Needs review
StatusFileSize
new 2613368-2.patch3.67 KB

Here's a patch that catches all parsing errors in Twig and YAML files.
Also filed #2613854: Don't parse third-party code: node_modules/, since that's where the first error is coming from.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Title: Handle parse errors gracefully » Handle Twig and YAML parse errors gracefully
Status: Needs review » Needs work
Issue tags: +Drupal 8 compatibility 
+++ b/potx.inc
@@ -1764,7 +1764,17 @@ function _potx_parse_js_string($string) {
+    watchdog('potx', "Twig parsing error on file @path: @error", array(
+        '@path' => $file,
+        '@error' => $e->getMessage(),
+    ));

@@ -2556,6 +2573,33 @@ function _potx_parse_shipped_configuration($save_callback = '_potx_save_string',
+    watchdog('potx', "YAML parsing error on file @path: @error", array(
+        '@path' => $yaml_path,
+        '@error' => $e->getMessage(),
+    ));

potx already has a mechanism to collect errors in files. I would use that, so we see those on the UI on localize.drupal.org for example and can help people debug problems. Otherwise the potx watchdog messages will just fly by...

herom’s picture

herom commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2613368-2-4.txt1.34 KB
new 2613368-4.patch4.13 KB

Replacing watchdog() calls with potx_error().

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Can we test this? We have assertions for errors reported in some of our tests, so I think we can test it on an invalid YAML / Twig?

herom’s picture

herom commented
Status: Needs work » Needs review
StatusFileSize
new 2613368-6-broken-yaml.test-only.patch213 bytes
new 2613368-6-broken-twig.test-only.patch218 bytes
new 2613368-6.patch4.55 KB

Let's try and see.

The last submitted patch, 6: 2613368-6-broken-twig.test-only.patch, failed testing.

herom’s picture

herom commented
StatusFileSize
new 2613368-8-broken-yaml.test-only.patch286 bytes
new 2613368-8.patch4.59 KB

Let's move the broken yaml into "config/install".

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

#8 looks good. Even if the YAML test only did not fail, I think its worth keeping a case for it :)

  • herom committed 7fed3ac on 7.x-3.x 
    Issue #2613368 by herom, Gábor Hojtsy, drumm: Handle Twig and YAML parse...
herom’s picture

herom commented
Status: Reviewed & tested by the community » Fixed

Great. Committed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.