flaky tests

[ronag]

FAIL  test/client.js
 ✖ (unnamed test)

  test/client.js                                          
  517 |       body                                        
  518 |     }, (err, { statusCode, headers, body }) => {  
> 519 |       t.error(err)                                
      | --------^                                         
  520 |       body                                        
  521 |         .on('data', () => {                       
  522 |           t.fail()                                

  test: test/client.js basic POST with empty stream
  stack: |
    test/client.js:519:9
    lib/api/api-request.js:132:14

  Error Origin:

  lib/llhttp/parser.js                                                                      
  187 |       const message = Buffer.from(inst.exports.memory.buffer, ptr, len).toString()  
  188 |       const code = constants.ERROR[err]                                             
> 189 |       return new HTTPParserError(message, code)                                     
      | -------------^                                                                      
  190 |     }                                                                               
  191 |   }                                                                                 
  192 | }                                                                                   

  stack: |
    Parser.execute (lib/llhttp/parser.js:189:14)
    Socket.onSocketData (lib/client.js:807:29)
  type: HTTPParserError
  code: HPE_INVALID_CONSTANT
  message: "HTTPParserError: "

 FAIL  test/client.js
 ✖ Cannot read property 'on' of undefined

  test/client.js                     
  519 |       t.error(err)           
  520 |       body                   
> 521 |         .on('data', () => {  
      | ---------^                   
  522 |           t.fail()           
  523 |         })                   
  524 |         .on('end', () => {   

  test: basic POST with empty stream
  stack: |
    test/client.js:521:10
    lib/api/api-request.js:132:14
  type: TypeError
  tapCaught: uncaughtException

[ronag]

Can't repro anymore.

[ronag]

Another flaky test:

 FAIL  test/redirect-request.js
 ✖ should be equal


  t.equal(statusCode, 200)
----^
  t.notOk(headers.location)
  /*

  --- expected    
  +++ actual      
  @@ -1,1 +1,1 @@ 
  -200            
  +426            

  test: test/redirect-request.js should follow redirections when going cross origin
  at:
    line: 293
    column: 5
    file: test/redirect-request.js
    type: Test
  stack: |
    Test.<anonymous> (test/redirect-request.js:293:5)

 FAIL  test/redirect-request.js
 ✖ should be equal

  */
  t.equal(body, 'POST')
----^
})

  --- expected      
  +++ actual        
  @@ -1,1 +1,1 @@   
  -POST             
  +Upgrade Required 

  test: test/redirect-request.js should follow redirections when going cross origin
  at:
    line: 306
    column: 5
    file: test/redirect-request.js
    type: Test
  stack: |
    Test.<anonymous> (test/redirect-request.js:306:5)

[ronag]

@dnlup see The above error, looks a bit like a parser error. The status message (Upgrade Required) somehow ended up in the message body.

[ronag]

Strangely it has only happened once so far... there seems to be some kind of interference between requests (parallel test runs?) and the WASM module.

Do we have any global state other than currentParser that might be causing issues?

Do we need to load a separate instance of the WASM module for each client instance?

[ronag]

In wasm_on_header_field, wasm_on_header_value and wasm_on_body. Is at relative to the global buffer inst.exports.memory.buffer or the instance buffer inst.exports.memory.buffer + this[kPtr]?

[ronag]

Feels related to #659. Do we have some form of state corruption somewhere?

[dnlup]

@dnlup see The above error, looks a bit like a parser error. The status message (Upgrade Required) somehow ended up in the message body.

There are some parser error indeed. This the first time I see them though🤔.

[dnlup]

Strangely it has only happened once so far... there seems to be some kind of interference between requests (parallel test runs?) and the WASM module.

Do we have any global state other than currentParser that might be causing issues?

Do we need to load a separate instance of the WASM module for each client instance?

I'll check about the global state, before we had the Map to keep track of instances, but I doubt that's the problem. I'll see if loading different instances can bring some benefits.

[ronag]

Another one:

 FAIL  test/abort-controller.js
 ✖ should be equal

  test/abort-controller.js                                               
  65 |         })                                                        
  66 |         response.body.on('end', () => {                           
> 67 |           t.equal('hello', Buffer.concat(bufs).toString('utf8'))  
     | ------------^                                                     
  68 |         })                                                        
  69 |       })                                                          
  70 |                                                                   

  --- expected      
  +++ actual        
  @@ -1,1 +1,1 @@   
  -Upgrade Required 
  +hello            

  test: test/abort-controller.js Abort native-abortcontroller before sending
    request (no body)
  stack: |
    RequestResponse.<anonymous> (test/abort-controller.js:67:13)

[dnlup]

In wasm_on_header_field, wasm_on_header_value and wasm_on_body. Is at relative to the global buffer inst.exports.memory.buffer or the instance buffer inst.exports.memory.buffer + this[kPtr]?

I am not sure

[dnlup]

Strangely it has only happened once so far... there seems to be some kind of interference between requests (parallel test runs?) and the WASM module.
Do we have any global state other than currentParser that might be causing issues?
Do we need to load a separate instance of the WASM module for each client instance?

I'll check about the global state, before we had the Map to keep track of instances, but I doubt that's the problem. I'll see if loading different instances can bring some benefits.

Expanding on this, I don't think we need different WASM instances. That is the reason why the parser reference is passed to every function call.

[ronag]

I hope #687 solved this.