Have supported browsers reject promises if a request takes too long #159
Conversation
Firefox and PhantomJS have their own timeout handlers Implemented a manual timeout handler for Chrome interface (since CDP is proxying to Chrome via remote protocols) Wrote test
KevinGrandon
left a comment
KevinGrandon
left a comment
There was a problem hiding this comment.
This looks great! Can we add yarn.lock in another diff? This package is still old school and most folks have just been using NPM. Happy to consider yarn, but if we can isolate that to another diff that would be ideal.
Left some other comments, but I think the only thing we need to really do is make sure CI is passing. I'm not sure why this broke other tests, but I can help look later too.
| * Injects javascript and other things we need. | ||
| */ | ||
| onOpen () { | ||
| console.log('on open called') |
There was a problem hiding this comment.
Can we remove for the time being? I would like a way for more verbose logs, but I want to allow people to opt into it.
| handler: function (req, reply) { | ||
| setTimeout(function() { | ||
| reply('Finally responding.') | ||
| }, 100000) |
There was a problem hiding this comment.
I'm wondering if this route should take a timeout in the query string? Then maybe we can do other things with it.
| const raceServerResponse = setTimeout(() => { | ||
| // If this setTimeout beat the error response, we're waiting too long | ||
| flag = false | ||
| }, 90000) |
There was a problem hiding this comment.
Is this still deterministic if we make this 30s or so? Ghost CI doesn't have great parallelism because we just use travis, so I am still a bit conscious on run time. Just wondering if we can use a shorter time here.
|
@KevinGrandon I'll look into tests failing and your other comments tonight. I might have configured the server routes incorrectly. |
2 participants
When running integration test, if a socket hung up or a request was taking way too long, we would have to wait the full 5 minute timeout before the test runner would return. This will make it so that we wait a reasonable amount of time before rejecting the promise and ending the test.