Virtual network part 2

[ef4]

No description provided.

[github-actions]

Test Results

584 tests  ±0   580 ✔️ ±0   8m 8s ⏱️ -38s
    1 suites ±0       4 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 34bfedb. ± Comparison against base commit b89827b.

♻️ This comment has been updated with latest results.

[jurgenwerk]

@ef4 @lukemelia an update:

1. Cleaned up the concepts of resolving, unresolving, url mapping from the loader (basically what Virtual network part 3 #1115 is trying to achieve - it seemed more convenient to do it here in this branch)

2. While hosts and realm tests were passing, the realm dom tests were failing (this could easily be reprouced by going to http://localhost:4201 which was broken because the browser was trying to load assets from an unresolved url, i.e. cardstack.com/base). It was happening because the realm server middleware is doing redirections (rootRealmRedirect, assetRedirect) and in that logic it wants to know the resolved URL for the realm. For that reason I changed virtual network's resolveURLMapping from private to public, and used it in redirection logic: 6f74533

I feel that this change might not be aligned with the vision for the virtual network so I'd like to discuss this. Perhaps the realm server should not know about resolved urls and the mentioned redirection logic should be moved to the virtual network as handlers?

3. At this point there are still test failures, this time in matrix tests. There seems to be an issue with fetching rooms from the matrix server where it doesn't return the chat room. It's puzzling how this is related to this refactor but I am checking it.

[jurgenwerk]

Follow up comments to issue described in (2), did some discussing with @lukemelia–a couple of ideas to make the resolveURLMapping back to being a private method:

1. There could be an AssetRedirector that would receive the mapping in the constructor, and have a method to be plugged in the middleware (similarly to assetRedirect). It would respond with the real url in the location header by figuring it out from the url mapping config rather than calling resolveURLMapping

2. We could explore mounting a handler for the redirection logic (as opposed to doing that in the koa middleware), mount it to virtual network, and add functionality in handle which would add special treatment if the response results in a redirect (making the location header url real, not virtual)

[jurgenwerk]

This PR leverages the virtual network in such a way that:

1. All URL mapping is moved to the virtual network which is now fully responsible for mapping (also known as resolving, reverse resolving realm urls)
2. All logic that depended on knowing the resolved url was moved to the virtual network
3. Redirections (asset redirection, realm root redirection) were moved out of the realm server (to satisfy (1))

This refactor is an intermediate step towards having isolated loaders that share a virtual network.

[habdelra]

nice!

[jurgenwerk]

I've been trying to see if I can get rid of body stream reading to create a new request with a changed url but to no avail...

1. Object.defineProperty(request, 'url', { value: mappedUrl }) will not work because the underlying url value really is immutable. Here is a demonstration - the request will still try to reach the original url even when trying to override it with defineProperty:

2. I saw some hints online that tee could be used to copy the body, like so:

let copiedBody = null;
  if (originalRequest.body) {
    copiedBody = originalRequest.body.tee()[0];
  }

  return new Request(url, {
    method: originalRequest.method,
    headers: originalRequest.headers,
    body: copiedBody,
    referrer: originalRequest.referrer,
    referrerPolicy: originalRequest.referrerPolicy,
    mode: originalRequest.mode,
    credentials: originalRequest.credentials,
    cache: originalRequest.cache,
    redirect: originalRequest.redirect,
    integrity: originalRequest.integrity,
  });

But again, the error persists:

3. clone()-ing the request and taking its body won't work either (failing with the same error):

return new Request(url, {
    method: originalRequest.method,
    headers: originalRequest.headers,
    body: originalRequest.clone().body,
    referrer: originalRequest.referrer,
    referrerPolicy: originalRequest.referrerPolicy,
    mode: originalRequest.mode,
    credentials: originalRequest.credentials,
    cache: originalRequest.cache,
    redirect: originalRequest.redirect,
    integrity: originalRequest.integrity,
  });

It is interesting because Safari gives a different error message:

4. But what if we try adding the duplex property?

let requestInit: RequestInit = originalRequest.clone();

  if (originalRequest.body) {
    requestInit.duplex = 'half';
  }

  return new Request(url, requestInit);

The request will fail with net::ERR_H2_OR_QUIC_REQUIRED which is indicative of missing http2 support on the server:

Moreover, TypeScript doesn't recognize duplex property on the Request - looks like this is more of an experimental thing for now...

[habdelra]

Another thought would be to Proxy the request:

let proxiedRequest = new Proxy(originalRequest, {
  get(target, property, received) {
    if (property === 'url') {
      return mappedURL;
    }
    return Reflect.get(target, property, received);
  }
})