unregister "unhandled" event when socket is closed

[serkanyersen]

This PR contains:

• bugfix
• feature
• refactor
• tests
• documentation
• metadata

Breaking Changes?

• yes
• no

If yes, please describe the breakage.

Please Describe Your Changes

I'm unsubscribing from this event whenever the socket is closed.

This addresses the Memory Leak warning mentioned in #223

[shellscape]

Please explain in detail why this is needed.

Ah, you've put it at the top. Please give attention to PR templates, I wasn't expecting there to be info outside of the Describe Your Changes section at the bottom. We should definitely add a test for this to protect against regressions.

[serkanyersen]

I'll fix the description and see if i can add a test for this.

[shellscape]

@serkanyersen ping

[shellscape]

@serkanyersen last ping before I mark this one as abandoned

[serkanyersen]

Hi, I just checked this again, but I'm not sure what kind of test can be written for this 🤔 I've checked the https://github.com/shellscape/webpack-plugin-serve/blob/master/test/routes.test.js but that test appears to be empty, to test this one would have to start the server, reload the page 5 to 10 times and some how read the warning [not] thrown by nodejs.

codecov shows that coverage for routes.js has dropped, this is possibly due to other tests that use this code, but I'm not sure how to explore this path, it might be as simple as remaking the request to localhost one more time in a test for example in https.test.js . I'm looking for your guidance on this one.

This fix is definitely needed though, we have been using this as a patch since May and no issues have been reported by the team, so it seems safe. Let me know how you want to proceed.

[Splurov]

We also experienced this problem and would be happy when it will be merged to master and released.

[shellscape]

@Splurov perhaps you could assist in creating a test for this change

[nicksrandall]

@shellscape I'll write some tests for this feature if you will give some guidance on how you'd like it to be setup. At first glance, it appears the that "routes" part of this lib has no tests at all.

[shellscape]

Ah shit, yeah. I got lazy with the tests for routes, and when I made that comment about adding tests, I didn't put two and two together. No worries, we'll YOLO this out next week and see how it goes.