Amphtml: gulp: "Uncaught SyntaxError" during rebuild

[choumx]

When gulp rebuilds due to a detected source change, it sometimes results in a syntax error. My guess is this is a race with applying AMP config somewhere.

CLI output typically looks like this:

[12:47:19] Compiled amp.js (1.220 s)
[12:47:19] Compiled amp.js (2.717 s)
[12:47:19] Applied AMP config (prod, localDev) to amp.js
[12:47:19] Applied AMP config (prod, localDev) to amp.js

I typically terminate gulp and restart to fix, but this means I'll have to wait for all extensions that have been lazily built to rebuild again.

/cc @ampproject/wg-infra

[rsimha]

@choumx From the CLI output, that's clearly a double-save resulting in a race. I'll look into debouncing the watch functionality of gulp so inadvertent double-saves result in a single rebuild operation.

[rsimha]

This should be fixed by #27738.

[rsimha]

@choumx still seeing a double save?

Did you check your local VSCode settings as described in https://github.com/ampproject/amphtml/pull/27738#issuecomment-613606564?

[choumx]

Yea, I actually deleted my local settings.json. I think @samouri was seeing this too.

[rsimha]

Can y’all post details for which IDE you’re using, what your local settings look like, what build steps you’re running, and what the gulp logs look like? I’m certain this is due to a double-save, but am at a loss for why your workflow results in one.

[Enriqe]

IDE: VSCode

Local settings:

// Place your settings in this file to overwrite the default settings
{
  "workbench.statusBar.visible": true,
  "workbench.activityBar.visible": false,
  "diffEditor.ignoreTrimWhitespace": false,
  "files.trimTrailingWhitespace": true,
  "editor.rulers": [80],
  "explorer.confirmDragAndDrop": false,
  "editor.tabSize": 2,
  "files.insertFinalNewline": true,
  "window.zoomLevel": 0,
  "workbench.editor.showTabs": true,
  "terminal.integrated.rightClickBehavior": "copyPaste",
  "editor.fontSize": 14,
  "terminal.integrated.fontSize": 14,
  "editor.minimap.enabled": false,
  "files.exclude": {
    "**/amp-story/0.1/**": true,
    "**/bower-components": true,
    "**/dist": true,
    "**/node_modules": true,
    "**/tmp": true
  },
  "files.watcherExclude": {
    "**/bower_components/**": true,
    "**/dist/**": true,
    "**/tmp/**": true
  },
  "eslint.alwaysShowStatus": true,
  "[json]": {
    "editor.defaultFormatter": "esbenp.prettier-vscode"
  },
  "[html]": {
    "editor.defaultFormatter": "esbenp.prettier-vscode"
  },
  "editor.codeActionsOnSave": {
    "source.fixAll.eslint": true
  },
  "[javascript]": {
    "editor.defaultFormatter": "esbenp.prettier-vscode"
  }
}

Build steps:

1. Running gulp
2. Visiting local sample which includes exensions
3. Extensions are lazily-built
   Error log:

[18:52:28] Using gulpfile ~/amphtml/gulpfile.js
[18:52:28] Starting 'default'...
[18:52:28] All packages in node_modules are up to date.
[18:52:28] Running gulp. Press Ctrl + C to cancel...
[18:52:28] Building version 2004282247000 of the runtime with the prod AMP config.
[18:52:28] ⤷ Use --config={canary|prod} with your gulp command to specify which config to apply.
[18:52:28] Running the default gulp task.
[18:52:28] ⤷ JS and extensions will be lazily built when requested from the server.
[18:52:28] Pre-building extension(s): amp-loader, amp-auto-lightbox
[18:52:28] ⤷ Use --extensions=amp-foo,amp-bar to choose which extensions to pre-build.
[18:52:28] ⤷ Use --extensions=inabox to pre-build just the extensions needed to load AMP ads.
[18:52:28] ⤷ Use --extensions_from=examples/foo.amp.html to pre-build just the extensions needed to load foo.amp.html.
[18:52:31] Recompiled all CSS files into build/ (3.038 s)
[18:52:32] Compiled Jison parsers into build/parsers/ (1.258 s)
[18:52:32] Bootstrapped 3p frames into dist.3p/current/ (019 ms)
[18:52:32] Running serve. Press Ctrl + C to cancel...
[18:52:33] Started AMP Dev Server at http://localhost:8000
[18:52:33] Serving unminified JS
GET /examples/example-sw.js 304 2.386 ms - -
[18:52:49] Compiled amp.js (4.016 s)
[18:52:55] Compiled amp-mustache-0.2.max.js → amp-mustache-latest.max.js (2.454 s)
[18:52:55] Compiled amp-story-auto-ads-0.1.max.js → amp-story-auto-ads-latest.max.js (309 ms)
[18:52:55] Compiled amp.js (1.838 s)
[18:52:55] Compiled amp-video-0.1.max.js → amp-video-latest.max.js (686 ms)
GET /dist/v0/amp-mustache-latest.max.js 200 18329.032 ms - 284255
GET /examples/amp-story/img/AMP-Brand-White-Icon.svg 200 2.363 ms - 1309
GET /dist/v0/amp-video-0.1.max.js 200 17413.157 ms - 417183
GET /dist/v0/amp-story-auto-ads-0.1.max.js 200 17408.656 ms - 527014
GET /dist/v0/amp-mustache-latest.max.js 200 1.157 ms - 284255
GET /dist/v0/amp-video-0.1.max.js 200 0.952 ms - 417183
GET /examples/amp-story/img/AMP-Brand-White-Icon.svg 200 0.919 ms - 1309
GET /dist/v0/amp-story-auto-ads-0.1.max.js 200 1.173 ms - 527014
GET /dist/v0/amp-mustache-0.2.max.js.map 200 3.450 ms - 616854
GET /dist/v0/amp-video-0.1.max.js.map 200 1.666 ms - 894224
GET /examples/example-sw.js 304 5.104 ms - -
GET /dist/v0/amp-story-auto-ads-0.1.max.js.map 200 4.588 ms - 1131834
(node:89260) UnhandledPromiseRejectionWarning: Error: Found 2 AMP_CONFIG(s) before write. Aborting!
    at sanityCheck (/Users/artezan/amphtml/build-system/tasks/prepend-global/index.js:54:11)
    at /Users/artezan/amphtml/build-system/tasks/prepend-global/index.js:178:7
    at async compileJs (/Users/artezan/amphtml/build-system/tasks/helpers.js:497:12)
(node:89260) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:89260) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
(node:89260) UnhandledPromiseRejectionWarning: Error: Found 2 AMP_CONFIG(s) before write. Aborting!
    at sanityCheck (/Users/artezan/amphtml/build-system/tasks/prepend-global/index.js:54:11)
    at /Users/artezan/amphtml/build-system/tasks/prepend-global/index.js:178:7
    at async compileJs (/Users/artezan/amphtml/build-system/tasks/helpers.js:497:12)
(node:89260) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 2)

A workaround that I've found for this is running gulp, then visiting localhost:8000, waiting for amp to finish building, and THEN visit sample page that causes extensions to be lazily-built.

This only works for some time, though. At some point, when I make local changes, save, reload the Chrome window, I get the Unexpected token ']' in the Chrome dev tools console and the loading hangs. At which point I restart the server and follow the steps mentioned here.

Since you mentioned the editor.codeActionsOnSave setting on the editor might be causing a double save I'm going to delete that and report back :)

[choumx]

I deleted everything in my local settings.json and still intermittently get Uncaught SyntaxError: Unexpected token ']' after save/rebuild.

[rsimha]

Thanks for the additional info, folks. Here's my analysis of the situation.

With #27738, I'm certain we've eliminated double builds due to hitting Ctrl+S more than once, via the debounce logic.

In @Enriqe's logs (https://github.com/ampproject/amphtml/issues/27722#issuecomment-621200908), I noticed that the local example page is being fetched _before_ amp.js is pre-built during server startup. IIRC, the core runtime uses some special options, and therefore doesn't currently use the double-build detection mechanism during pre-build, causing it to also be lazy-built:

https://github.com/ampproject/amphtml/blob/67815dc7b6c1f434a09fcd52227a01f7b0b2c728/build-system/server/lazy-build.js#L51-L75

I'll look into un-special-casing the core runtime to eliminate the pre-build + lazy-build code path, after which I think this problem should go away. Stay tuned.

[rsimha]

@choumx @Enriqe I was able to reproduce the problem (loading the example page before pre-build was triggering an additional lazy-build). This is now fixed via #28106.

[choumx]

Thanks for looking into this again! I suspect there may be other triggers since I've noticed this after only a few save/rebuilds, but I'll reopen with more details if I come across it again.

[Enriqe]

Thanks for looking into this @rsimha!