1816253 - Failures with various HTTP responses (301, 204, etc) in WebTransport wpt connect tests

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect, P2)

Description

[Randell Jesup [:jesup] (needinfo me)]

There are a number of failures with the connect.https.any.js tests:

18:09.00 TEST_END: Test ERROR, expected OK. Subtests passed 1/7. Unexpected 6
FAIL WebTransport session is established with status code 204 - promise_test: Unhandled rejection with value: object "WebTransportError: WebTransport connection rejected"FAIL WebTransport session establishment fails with status code 301 - assert_true: expected true got false
@https://web-platform.test:8443/webtransport/connect.https.any.js:23:14
FAIL WebTransport session establishment with status code 401 - assert_true: expected true got false
@https://web-platform.test:8443/webtransport/connect.https.any.js:33:14
FAIL WebTransport session establishment fails with status code 404 - assert_true: expected true got false
@https://web-platform.test:8443/webtransport/connect.https.any.js:43:14
FAIL Echo back request headers - assert_equals: expected (string) "https://web-platform.test:8443" but got (undefined) undefined
check_and_remove_standard_headers@https://web-platform.test:8443/webtransport/resources/webtransport-test-helpers.sub.js:68:16
@https://web-platform.test:8443/webtransport/connect.https.any.js:63:36
FAIL Cookie header is not echoed back - assert_equals: expected (string) "https://web-platform.test:8443" but got (undefined) undefined
check_and_remove_standard_headers@https://web-platform.test:8443/webtransport/resources/webtransport-test-helpers.sub.js:68:16
@https://web-platform.test:8443/webtransport/connect.https.any.js:86:36
ERROR /webtransport/connect.https.any.serviceworker.html - Unhandled rejection: WebTransport connection rejected

Ed/Sunil: you may want to take a look at these

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Output with WebTransport:5,nsWebTransport:5,DataPipe:5,timestamp

Comment 2

[Sunil Mayya]

Thanks for sharing the bug Randell!. Ed and I will discuss between us and will look into this.

Comment 3

[Sunil Mayya]

Here is the list of issues that I discovered with connect tests related to 301/401/404

• Issue 1

  • For 301/401 we are supposed to reject these promises.
  • However, as we do not read any return values on session closure Htt3Session, we treat session close as success and do not propagate the error to the resolver. Hence, these two asserts which expects the wt.ready and wt.closed promises to be rejected fails.
  • Ed has a patch for review to fix this

• Issue 2

  • Currently we reject only ready promises for session failures. We are not rejecting closed promise here This leads to the following assertion failues. The fix is straightforward.

• Issue 3

  • With Issue 1, and Issue 2 fixed we have one more problem where the tests expect exception of type WebTransportError to be thrown. However the test does not recognize it leading to failure here

For Issue 3 I prepared the following patch to ensure we throw WebTransportError object. This patch re-uses exiting code from the clean-up function and I expected it to work.

  void WebTransport::RejectWaitingConnection(nsresult aRv) {               

    MOZ_ASSERT(mState == WebTransportState::CONNECTING);                              
    mState = WebTransportState::FAILED;                     
    LOG(("Rejected connection %x", (uint32_t)aRv));    
                                                   
    RefPtr<WebTransportError> error = new WebTransportError(    
   ""_ns, WebTransportErrorSource::Session);          
    AutoJSAPI jsapi;                       
                                           
    if (!jsapi.Init(mGlobal)) {    
      MOZ_ASSERT(false);    
      return;                                             
    }                        
    JSContext* cx = jsapi.cx();                   
    JS::Rooted<JS::Value> errorValue(cx);                       
    bool ok = ToJSValue(cx, error, &errorValue);    
    if (!ok) {    
      MOZ_ASSERT(false);    
      return;                        
    }                                                                               
                                                                      
    // https://w3c.github.io/webtransport/#webtransport-internal-slots    
    // "Reliability returns "pending" until a connection is established" so    
    // we leave it pending                                                       
    mClosed->MaybeReject(errorValue);    
    mReady->MaybeReject(errorValue);    
    // This will abort any pulls for IncomingBidirectional/UnidirectionalStreams    
    mIncomingBidirectionalPromise->MaybeResolveWithUndefined();    
    mIncomingUnidirectionalPromise->MaybeResolveWithUndefined();    
      
    // We never set mChild, so we aren't holding a reference that blocks GC    
    // (spec 5.8)    
  }

Randell, do you know if this (WebTransortError object propagation) is fully supported? Please let me know if I am missing something in my patch.

Comment 4

[Sunil Mayya]

With Randell's latest patch all the above issues has been resolved.

We are just now seeing the following issues:

Echo back request headers
Cookie header is not echoed back

The above issue is related to origin header missing from the request which is already tracked by Bug 1817560

Comment 5

[Sunil Mayya]

All issues with regard to connect tests are resolved.