Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crash with url returning status Code != 200 #10

Closed
gustavopaes opened this issue Feb 4, 2017 · 7 comments
Closed

crash with url returning status Code != 200 #10

gustavopaes opened this issue Feb 4, 2017 · 7 comments
Assignees

Comments

@gustavopaes
Copy link
Contributor

@gustavopaes gustavopaes commented Feb 4, 2017

Using "/api" route with an url that return statusCode != 200, the server crashes with the following error message:

[AMPBench:v.1.0][2017-02-04T13:01:20.863Z] [validator-signature:96c9247f6c997613302517643841f1f96e17bcf14ca3d51ebffae43b6274e648][HTTP:401] /api https://passwordprotected.com/amp.htm
_http_outgoing.js:356
    throw new Error('Can\'t set headers after they are sent.');
    ^

Error: Can't set headers after they are sent.
    at ServerResponse.OutgoingMessage.setHeader (_http_outgoing.js:356:11)
    at ServerResponse.header (/home/gpaes/git/ampbench/node_modules/express/lib/response.js:719:10)
    at ServerResponse.send (/home/gpaes/git/ampbench/node_modules/express/lib/response.js:164:12)
    at ServerResponse.json (/home/gpaes/git/ampbench/node_modules/express/lib/response.js:250:15)
    at on_output (/home/gpaes/git/ampbench/ampbench_routes.js:442:29)
    at IncomingMessage.res.on (/home/gpaes/git/ampbench/ampbench_lib.js:926:21)
    at emitNone (events.js:91:20)
    at IncomingMessage.emit (events.js:185:7)
    at endReadableNT (_stream_readable.js:974:12)
    at _combinedTickCallback (internal/process/next_tick.js:74:11)

To fix it, it's necessary to prevent double on_output_callback call at ampbench_lib.js, at line 911 and 926. Maybe a return or else.

if (!http_response.statusIsOK()) { // NOT (200 == this.http_response_code)
    http_response.http_response_body = '';
    return on_output_callback(http_response, [CHECK_FAIL]); // !!! RETURN to front-end  - - - - - - - - - - - -
}
@pietergreyling pietergreyling self-assigned this Feb 4, 2017
@pietergreyling

This comment has been minimized.

Copy link
Collaborator

@pietergreyling pietergreyling commented Feb 4, 2017

@gustavopaes I am not seeing any server-side issues.
When I try:
https://ampbench.appspot.com/validate?url=https://passwordprotected.com/amp.htm
I get:

AMP Checks [FAIL] [URL is unreachable] [Server SSL Certificate rejected[Error: self signed certificate[DEPTH_ZERO_SELF_SIGNED_CERT]]]

and with:
https://ampbench.appspot.com/api?url=https://passwordprotected.com/amp.htm
the result is as follows:

{
    "status": "FAIL",
    "url": "https://passwordprotected.com/amp.htm",
    "http_response": {
        "url": "https://passwordprotected.com/amp.htm",
        "http_code": 0,
        "http_text": "Server SSL Certificate rejected[Error: self signed certificate[DEPTH_ZERO_SELF_SIGNED_CERT]]",
        "response_time_ms": 0,
        "redirected": false,
        "redirects_count": 0,
        "redirects_urls": [],
        "is_https": true,
        "is_https_cert_authorized": false,
        "is_https_ssl_error": "Error: self signed certificate[DEPTH_ZERO_SELF_SIGNED_CERT]"
    },
    "amp_links": {
        "canonical_url": "",
        "amphtml_url": "",
        "amp_uses_feed": false
    },
    "results": []
}
@gustavopaes

This comment has been minimized.

Copy link
Contributor Author

@gustavopaes gustavopaes commented Feb 4, 2017

@pietergreyling sorry, I changed at the error the original url request to this "passwordprotected.com" thinking this was not exist... but exists!

The problem is on the routes /api and /api2. And the page needs to return some statusCode >= 400, like these:

https://test-response-server.herokuapp.com/html/404
https://test-response-server.herokuapp.com/html/403
https://test-response-server.herokuapp.com/html/500

@nygellyndley

This comment has been minimized.

Copy link
Contributor

@nygellyndley nygellyndley commented Jun 20, 2017

Seeing the same issue.

This issue can be duplicated on Google's own(?) ampbench server : https://ampbench.appspot.com/api?url=https%3A%2F%2Fampbench.appspot.com%2Fxxx

Repeat requesting this results in a 502 bad gateway from ampbench.appspot.com which is going the app crashing and restarting.

@pietergreyling

This comment has been minimized.

Copy link
Collaborator

@pietergreyling pietergreyling commented Jun 26, 2017

Tested it as follows and it does not crash:
https://ampbench.appspot.com/api?url=https%3A%2F%2Fampbench.appspot.com%2Fxxx
returns this:

{
    "status": "FAIL",
    "url": "https://ampbench.appspot.com/xxx",
    "http_response": {
        "url": "https://ampbench.appspot.com/xxx",
        "http_code": 404,
        "http_text": "HTTP Status: 404 - Not Found",
        "response_time_ms": -1498490894790,
        "redirected": false,
        "redirects_count": 1,
        "redirects_urls": [
            "https://ampbench.appspot.com"
        ],
        "is_https": true,
        "is_https_cert_authorized": null,
        "is_https_ssl_error": ""
    },
    "amp_links": {
        "canonical_url": "",
        "amphtml_url": "",
        "amp_uses_feed": false
    },
    "results": []
}

And:
https://ampbench.appspot.com/api?url=https://ampbench.appspot.com
returns this:

{
    "status": "FAIL",
    "url": "https://ampbench.appspot.com",
    "http_response": {
        "url": "https://ampbench.appspot.com",
        "http_code": 200,
        "http_text": "HTTP Status: 200 - OK",
        "response_time_ms": 1,
        "redirected": false,
        "redirects_count": 1,
        "redirects_urls": [
            "https://ampbench.appspot.com"
        ],
        "is_https": true,
        "is_https_cert_authorized": true,
        "is_https_ssl_error": ""
    },
    "amp_links": {
        "canonical_url": "",
        "amphtml_url": "",
        "amp_uses_feed": false
    },
    "results": [
.....
    ]
}

Both are behaving correctly.

@gustavopaes

This comment has been minimized.

Copy link
Contributor Author

@gustavopaes gustavopaes commented Jun 26, 2017

Run it locally and you will see broking. At appspot the app restart after crash.

And the crash happens after response. That's is why you don't see any error at response.

$ npm start

> ampbench@1.0.0 start /home/gpaes/git/ampbench
> node ampbench_main.js

[AMPBench:v.1.0][2017-06-26T16:05:49.160Z] service started on host [:::8080]
[AMPBench:v.1.0][2017-06-26T16:05:52.482Z] [validator-signature:*unavailable*][HTTP:404] /api https://ampbench.appspot.com/xxx
[AMPBench:v.1.0][2017-06-26T16:05:52.571Z] [validator-signature:f138baf0dc4cf9b6125730ce418492c1ff8a15de4083861fd97d3060abe52f46][HTTP:404] /api https://ampbench.appspot.com/xxx
_http_outgoing.js:356
    throw new Error('Can\'t set headers after they are sent.');
    ^

Error: Can't set headers after they are sent.
    at ServerResponse.OutgoingMessage.setHeader (_http_outgoing.js:356:11)
    at ServerResponse.header (/home/gpaes/git/ampbench/node_modules/express/lib/response.js:719:10)
    at ServerResponse.send (/home/gpaes/git/ampbench/node_modules/express/lib/response.js:164:12)
    at ServerResponse.json (/home/gpaes/git/ampbench/node_modules/express/lib/response.js:250:15)
    at on_output (/home/gpaes/git/ampbench/ampbench_routes.js:445:29)
    at IncomingMessage.res.on (/home/gpaes/git/ampbench/ampbench_lib.js:948:21)
    at emitNone (events.js:91:20)
    at IncomingMessage.emit (events.js:185:7)
    at endReadableNT (_stream_readable.js:974:12)
    at _combinedTickCallback (internal/process/next_tick.js:74:11)

npm ERR! Linux 3.13.0-119-generic
npm ERR! argv "/usr/local/src/node-v6.9.1-linux-x64/bin/node" "/usr/bin/npm" "start"
npm ERR! node v6.9.1
npm ERR! npm  v3.10.8
npm ERR! code ELIFECYCLE
npm ERR! ampbench@1.0.0 start: `node ampbench_main.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the ampbench@1.0.0 start script 'node ampbench_main.js'.
npm ERR! Make sure you have the latest version of node.js and npm installed.
npm ERR! If you do, this is most likely a problem with the ampbench package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     node ampbench_main.js
npm ERR! You can get information on how to open an issue for this project with:
npm ERR!     npm bugs ampbench
npm ERR! Or if that isn't available, you can get their info via:
npm ERR!     npm owner ls ampbench
npm ERR! There is likely additional logging output above.

npm ERR! Please include the following file with any support request:
npm ERR!     /home/gpaes/git/ampbench/npm-debug.log
@pietergreyling

This comment has been minimized.

Copy link
Collaborator

@pietergreyling pietergreyling commented Oct 9, 2017

@gustavopaes
FYI, please see this commit which should fix this issue: Validator rollup + fix #10

@gustavopaes

This comment has been minimized.

Copy link
Contributor Author

@gustavopaes gustavopaes commented Oct 10, 2017

fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.