ci: fix errors in ci github action for node 8 and 9

[inigomarquinez]

This PR fixes nyc version to 14.1.1 when running tests in node 8 or node 9. nyc 15.x requires a yargs package version that requires node >=10.

I've also added the latest versions of node (18, 19 and 20) to the matrix.

Related to jshttp/http-errors#108

Warning

The CI is failing for one test in node@21 and node@22. The error has nothing to do with this PR, because those node versions were not tested before and with this PR we discovered it.

[UlisesGascon]

Can we include Node@21?

[wesleytodd]

Yeah agree we should add node 21, but everything else looks good!

[inigomarquinez]

Added support for node@21 here

[inigomarquinez]

The CI is failing for one test in node@21 and node@22. The error has nothing to do with this PR, because those node versions were not tested before and with this PR we discovered it.

[wesleytodd]

Interesting, I don't have time to look into the failure right now, but the test is pretty simple and clear so likely this is a change in node as I don't think this package ever returns a 400, so likely we either need to update/remove this test.

[inigomarquinez]

@UlisesGascon , @wesleytodd

How should we proceed with this PR? Can we merge it and then fix the failing test in a separate PR? Or do you want to proceed differently?

[wesleytodd]

So I just pulled this down and am poking around at it, and it is because node is sending back the 400 before it gets passed onto finalhandler. I was looking for a clear commit which would cause it but could not find it, but my guess is maybe the llhttp update? @ShogunPanda, would you mind taking a look to see if this was an expected breaking change?

[wesleytodd]

I realized the thread here didn't have enough context, sorry about that, here is what I was hoping you could look at @ShogunPanda:

The test passes an invalid path of /foo%20§. In node versions prior to 21 this was passed along to finalhandler and was not considered a 400 from node. Now it is returning a 400 with an empty body which circumvents finalhandler's 404 and html response.

[ShogunPanda]

@wesleytodd Yes, the llhttp is the "problem". Between 8 and 9 llhttp became strict by default: invalid characters in the URL are not supported anymore and return in a parse error.

[wesleytodd]

Awesome, thanks for the direct confirmation. Now that we are sure that is the cause, I think we can just remove these tests for invalid paths since we will also be dropping node support prior to this soon and I dont think we have any other changes slated for this package which warrant trying to conditionally fix the test.

@inigomarquinez do you want to do that in this PR to get it passing before we merge? Or should we do it in a separate one? I am not super opinionated either way.