ignore protocol error from closing tab after Chrome has been killed

[brendankenny]

fixes #1583

[patrickhulce]

This seems to just ignore logging is there not an underlying issue to resolve?
Also won't this not log the scenario Eric was describing in that issue too which seems legitimate, no? Or maybe I'm just totally misunderstanding what the goal is here, haha.

[brendankenny]

This seems to just ignore logging is there not an underlying issue to resolve?

Eric's error is actually "Unable to load the page. Please verify the url you are trying to review." Everything above it is actually just the log statement (since it's logging the error object (including stack) on master, not e.message).

The problem comes from the exception bubbling up and doing chromeLauncher.kill() faster than the close/${pageid} command can be sent, so the protocol ends up trying to close a tab in a Chrome that isn't there anymore.

So, the error is right, the logging is just confusing things in this case.

[patrickhulce]

Ah, so the only issue was logging it!

[brendankenny]

yeah, it's a little weird because GatherRunner.disposeDriver() is purposefully outside the promise chain so we aren't blocking while the driver disconnects. We thought we should add some logging in #1543 so we'd still notice bad things, but I think this is arguably not a bad thing.

We could get trickier and not exit until we've disconnected or not disconnect if we're going to exit, but if we've already exited Chrome I personally think we just don't care if the close tab command was issued before Chrome closed but too late to do anything.

[ebidel]

Hilarious. So this does actually does fix the error? It does appear it's just not logging a message. Maybe you can add comment?

Can a test check this?

[brendankenny]

I did add a comment :) More of a comment?

I'm not sure we need a test. It's just suppressing the log when closing the tab in a browser that's already been closed. In a sense, it was already successful :)

[ebidel]

Maybe just link to #1583? It has all the details.

[brendankenny]

Maybe just link to #1583? It also all the details.

done