fix(web-server): ignore future socket errors

[mgroenhoff]

Karma sometimes reports uncaught exceptions (ECONNRESET) which causes it to exit the process with a non-zero exit code.

Fixes #3295

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[karmarunnerbot]

✅ Build karma 62 completed (commit 6b00be646c by @mgroenhoff)

[AppVeyorBot]

✅ Build karma 2460 completed (commit 6b00be646c by @mgroenhoff)

[mgroenhoff]

@googlebot I signed it!

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[karmarunnerbot]

✅ Build karma 61 completed (commit 6b00be646c by @mgroenhoff)

[johnjbarton]

Seems like this will cause all errors to be ignored after we start the two-way socket-io.

The spurious errors from #3295 only occur after the run completes. Isn't the time to turn off errors?

[mgroenhoff]

I would say that you don't want those socket errors to crash the entire Karma process and this prevents just that. It still allows for the upgrade handler(s) to attach their own error event handlers.

Maybe I should change the comment so it says exactly that: that it only prevents the process from crashing.

[mgroenhoff]

In fact. I think this is somewhat similar to what Node.js does when it's HTTP server receives a new connection. It attaches an error handler to prevent it from going down. It forwards it to the 'clientError' event on the server, which when emitted does not crash if it does not have any handlers. But in the case if an upgrade request it totally lets go of the socket and removes that error handler so the socket is entirely yours.

[johnjbarton]

Sorry I missed your reply.

Yes, just change the comment.

[devoto13]

The spurious errors from #3295 only occur after the run completes. Isn't the time to turn off errors?

I agree. While this fixes the problem it will swallow errors in other situations. At the bare minimum, it should log an error as a warning similar to https://github.com/karma-runner/karma/blob/master/lib/server.js#L145. Ideally, it should also reset the handler after the complete event is received to target specifically #3295, where we don't care about the browser errors anymore.

[AppVeyorBot]

✅ Build karma 2783 completed (commit 8f39f90852 by @mgroenhoff)

[karmarunnerbot]

✅ Build karma 386 completed (commit 8f39f90852 by @mgroenhoff)

[karmarunnerbot]

✅ Build karma 385 completed (commit 8f39f90852 by @mgroenhoff)

[costescuandrei]

Any update on this? Is anyone who knows the code willing to do the required review?

[johnjbarton]

I reviewed but never received updates.

[jginsburgn]

@mgroenhoff do we still need to merge this PR?