ampproject / ampproject/amp-toolbox

Optimizer crashes on non-HTML input

Open
#557 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
HTML
Stars
459
Forks
242
PR merge metrics
No merged PRs in 30d

Description

I'm not entirely sure why this is happening. But I thought I'd report it because it could be an easy fix.

In the AMP Camp project, all server output is run through amp-optimizer, whether that output is HTML or JSON. This is probably a bad idea. Regardless, when that JSON is returned to a request from an AMP page, the JSON sails through without a hitch. But when I make the API request from the browser, the optimizer dies.

Here's a sample such request:

`https://camp.samples.amp.dev/api/product?productId=L1130&List=true&__amp_source_origin=https%3A%2F%2Fcamp.samples.amp.dev`

It usually would return JSON that looks like this:
```javascript
{ items:
{ Main_Id: 'L1030',
Product_Title: 'Outdoor Research Echo Long Sleeve Hoody - Women\'s',
Product_URL:
'https://www.campmor.com/c/outdoor-research-echo-long-sleeve-hoody---womens',
Brand: 'Outdoor Research',
Photo:
'https://static.campmor.com/wcsstore/Campmor/static/images/items/main/L1030.jpg',
Price: '65.00',
Discount_Price: '65.00',
Low_Price: '65.00',
High_Price: '65.00',
Savings: null,
All_Colors: [ [Object] ],
All_Sizes: [ [Object] ],
Description:
'Relief from the sun is essential during exposed activities. The Outdoor Research Women?s Echo Hoody is the perfect balance of performance and ultralight protection.? This long sleeve hoody is built with UPF sun protection, Airvent? moisture management, ActiveFresh? odor control, and topped off with an anti-chafing flat seam design Airvent? 100% Polyester fabric ActiveFresh? active odor control Wicking UPF 15 sun protection shields from harmful effects of sun exposure Flat-seam construction for a comfortable, low-profile, chafe-free fit Raglan sleeves Fold over mitt cuffs Reflective logo Center back length: 27 1/4 in Avg. weight: 3.6 oz (102g) size M Trim fit',
Default_Color: 'SGR',
Default_Color_Id: 'L1030SGRS',
Default_Size: 'S',
Default_Size_Id: 'L1030SGRS',
ReviewCount: 17,
ReviewFullStars: [ 1, 1, 1, 1 ],
ReviewEmptyStars: [ 1 ],
DefaultColor: 'SGR',
UniqueColor: true,
DefaultSize: 'S',
DefaultPrice: '65.00',
DefaultQuantityDisabled: false } }
```

Instead, though, I get this stack trace:

```
(node:17195) UnhandledPromiseRejectionWarning: TypeError: this.html.charCodeAt is not a function
at Preprocessor.advance (/Users/morss/Sites/samples/amp-camp/node_modules/@ampproject/toolbox-optimizer/node_modules/parse5/lib/tokenizer/preprocessor.js:106:28)
at Tokenizer._consume (/Users/morss/Sites/samples/amp-camp/node_modules/@ampproject/toolbox-optimizer/node_modules/parse5/lib/tokenizer/index.js:292:34)
at Tokenizer.getNextToken (/Users/morss/Sites/samples/amp-camp/node_modules/@ampproject/toolbox-optimizer/node_modules/parse5/lib/tokenizer/index.js:253:29)
at Parser._runParsingLoop (/Users/morss/Sites/samples/amp-camp/node_modules/@ampproject/toolbox-optimizer/node_modules/parse5/lib/parser/index.js:422:42)
at Parser.parse (/Users/morss/Sites/samples/amp-camp/node_modules/@ampproject/toolbox-optimizer/node_modules/parse5/lib/parser/index.js:344:14)
at Object.parse (/Users/morss/Sites/samples/amp-camp/node_modules/@ampproject/toolbox-optimizer/node_modules/parse5/lib/index.js:10:19)
at TreeParser.parse (/Users/morss/Sites/samples/amp-camp/node_modules/@ampproject/toolbox-optimizer/lib/TreeParser.js:165:25)
at DomTransformer.transformHtml (/Users/morss/Sites/samples/amp-camp/node_modules/@ampproject/toolbox-optimizer/lib/DomTransformer.js:100:29)
at ServerResponse.res.send (/Users/morss/Sites/samples/amp-camp/dist/server/server.js:122:21)
at Request.request [as _callback] (/Users/morss/Sites/samples/amp-camp/dist/server/server.js:315:17)
```

/cc @demianrenzulli @antoinebisch

Contributor guide

Open the contributing guide

Research direction

Reproduce the browser API request from the issue and start with the stack trace at dist/server/server.js, then follow DomTransformer.js and TreeParser.js into the optimizer call. Compare the HTML and JSON response paths. Done means the non-HTML response no longer crashes and the JSON request completes successfully.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, node.js
Domain
api, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.