Solve issues for Node.js CITGM

[UlisesGascon]

Hi team! I was researching a bit about the current issues with CITGM, there is a PR https://github.com/nodejs/citgm/pull/977 open to re-add express, but one of the tests is failling.

How to reproduce?

1. Use Node 20.x (recommended 20.11.x) nvm use 20.11.0
2. Install cigtm globally npm i -g citgm
3. Run the tests for express citgm express

What is the error?

info: starting            | express             
info: lookup              | express             
info: lookup-notfound     | express             
info: lookup-githead      | https://github.com/expressjs/express/archive/8368dc178af16b91b576c4c1d135f701a0007e5d.tar.gz
info: express npm:        | Downloading project: https://github.com/expressjs/express/archive/8368dc178af16b91b576c4c1d135f701a0007
info: express npm:        | Project downloaded 8368dc178af16b91b576c4c1d135f701a0007e5d.tar.gz
info: express npm:        | npm install started 
info: express npm:        | npm install successfully completed
info: express npm:        | test suite started  
error: failure             | The canary is dead: 
error: failing module(s)   |                     
error: module name:        | express             
error: version:            | 4.18.2              
error: error:              | The canary is dead: 
error: error:              | undefined                                                                                     
error:                     | added 432 packages in 36s                                                                     
error:                     |                                                                                               
error:                     | > express@4.18.2 test                                                                         
error:                     | > mocha --require test/support/env --reporter spec --bail --check-leaks test/ test/acceptance/
error:                     |                                                                                               
error:                     |          
[...redacted...]
error:                     | 350 passing (636ms)                                                                                                                                                
error:                     | 1 failing                                                                                                                                                          
error:                     |                                                                                                                                                                    
error:                     | 1) express.json()                                                                                                                                                  
error:                     | with strict option                                                                                                                                                 
error:                     | when undefined                                                                                                                                                     
error:                     | should 400 on primitives:                                                                                                                                          
error:                     | Error: expected `[entity.parse.failed] Unexpected token 't', "#rue" is not valid JSON` response body, got `[entity.parse.failed] Unexpected token 't', "#" is not v
error:                     | at Context.<anonymous> (test/express.json.js:265:12)                                                                                                               
error:                     | at process.processImmediate (node:internal/timers:478:21)                                                                                                          
error:                     | ----                                                                                                                                                               
error:                     | at error (node_modules/supertest/lib/test.js:335:15)                                                                                                               
error:                     | at Test._assertBody (node_modules/supertest/lib/test.js:206:16)                                                                                                    
error:                     | at /private/var/folders/7h/550p8fxd2gn4vwn6x_q9stdh0000gn/T/421860ca-ea70-4620-92e0-1d11e288e294/express/node_modules/supertest/lib/test.js:308:13                 
error:                     | at Test._assertFunction (node_modules/supertest/lib/test.js:285:13)                                                                                                
error:                     | at Test.assert (node_modules/supertest/lib/test.js:164:23)                                                                                                         
error:                     | at Server.localAssert (node_modules/supertest/lib/test.js:120:14)                                                                                                  
error:                     | at Object.onceWrapper (node:events:632:28)                                                                                                                         
error:                     | at Server.emit (node:events:518:28)                                                                                                                                
error:                     | at emitCloseNT (node:net:2279:8)                                                                                                                                   
error:                     | at process.processTicksAndRejections (node:internal/process/task_queues:81:21)                                                                                     
error:                     |                                                                                                                                                                    
error:                     |                                                                                                                                                                    
error:                     |                                                                                                                                                                    
error:                     |                                                                                                                                                                    
error:                     | npm notice                                                                                                                                                         
error:                     | npm notice New minor version of npm available! 10.2.4 -> 10.4.0                                                                                                    
error:                     | npm notice Changelog: <https://github.com/npm/cli/releases/tag/v10.4.0>                                                                                            
error:                     | npm notice Run `npm install -g npm@10.4.0` to update!                                                                                                              
error:                     | npm notice                                                                                                                                                         
error: done                | The smoke test has failed.
info: duration            | test duration: 49198ms

I was not able to reproduce this error while running the tests locally from a express clone, but after doing a bit of research seems like it is related to https://github.com/expressjs/body-parser/commit/368a93a613a1ac6cbdec9694f4018e707b3c1f50 ?

As a side note, I will update the CI to include the most recent Node.js versions:

• [x] https://github.com/expressjs/express/pull/5430
• [x] https://github.com/expressjs/express/pull/5429
• [x] https://github.com/expressjs/express/pull/5490

[dougwilson]

Yes, I fixed that a long time ago, also in this repo too, which is probably why you cannot reproduce it while running tests locally from clone?

[UlisesGascon]

Great point @dougwilson. Now I am thinking that maybe is because CITGM is using 4.18.2 and the patch in body-parser@1.20.2 is not yet released within express? 🤔

unreleased
==========

  * Fix routing requests without method
  * deps: body-parser@1.20.2
    - Fix strict json error message on Node.js 19+
    - deps: content-type@~1.0.5
    - deps: raw-body@2.5.2

4.18.2 / 2022-10-08
===================

  * Fix regression routing a large stack in a single route
  * deps: body-parser@1.20.1
    - deps: qs@6.11.0
    - perf: remove unnecessary object clone
  * deps: qs@6.11.0

[dougwilson]

Oh, interesting. I guess that is true, if citgm uses published vereion it wouldn't be there. I was confused bc I saw the log saying downloading from github.com. I guess we can queue up a parch release quickly to make it available to citgm, without going through the minor release timeline.

[UlisesGascon]

Yeah, a patch release sounds great 👍

[wesleytodd]

Could we maybe do a release as a group on video? One of the things we're gonna need to get better at is training people and how to do these things safely and with the correct procedure. This seems like a good one to get started with.

[dougwilson]

Oh, ok, good thing you mentioned! I wqs gonna try and squeeze it in today so we could get the citgm checked off, but I can hold off for us to find a time and day to do it together.

[wesleytodd]

Obviously we want to land the 5 release soonish, but other than that do we have anything pending to use as an example to record for educational purposes?

[dougwilson]

This was just to make a patch for 4 for citgm to use. Happy to do whatever. I have to leave in about 45 mins, and was on to do this patch before the work week starts and I get more limited, but happy to di whatever we want, just let me know. I'll keep holding until I hear more.

[wesleytodd]

To circle back, there was not time today to finish the CICD and also record this release. We will try and get a recording in a future release, so going to mark my comments and the rest as off topic. Sorry for delaying it.

[UlisesGascon]

Curent status

We are just waiting for this PR to be merged, the CI is passing 🥳

[UlisesGascon]

This is officially completed! Express.js is back to the CITGM! 🎉