Revert "Redoing "#2073: Update Request to support cache option" with compat flags"

src/workerd/api/http-test.js

@@ -126,7 +126,6 @@ export const inspect = {
     });
     assert.strictEqual(util.inspect(request),
 `Request {
-  cache: undefined,
   keepalive: false,
   integrity: '',
   cf: undefined,
@@ -206,36 +205,3 @@ export const inspect = {
     await messagePromise;
   }
 };
-
-export const cacheMode = {
-  async test() {
-    {
-      const req = new Request('https://example.org', { });
-      assert.strictEqual(req.cache, undefined);
-    }
-    {
-      const req = new Request('https://example.org', { cache: 'no-store' });
-      assert.strictEqual(req.cache, 'no-store');
-    }
-    {
-      const req = new Request('https://example.org', { cache: 'no-cache' });
-      assert.strictEqual(req.cache, 'no-cache');
-    }
-    assert.throws(() => {
-      new Request('https://example.org', { cache: 'unsupported' });
-    }, {
-      name: 'TypeError',
-      message: 'Unsupported cache mode: unsupported',
-    });
-
-    // Any value other than undefined is currently not supported
-    // TODO(soon): The no-store and no-cache values will be supported
-    // soon, at which time this test will need to be updated.
-    await assert.rejects((async () => {
-      await fetch('http://example.org', { cache: 'no-store' });
-    })(), {
-      name: 'TypeError',
-      message: 'Unsupported cache mode: no-store',
-    });
-  }
-};

src/workerd/api/http-test.wd-test

@@ -11,7 +11,7 @@ const unitTests :Workerd.Config = (
           ( name = "SERVICE", service = "http-test" )
         ],
         compatibilityDate = "2023-08-01",
-        compatibilityFlags = ["nodejs_compat", "service_binding_extra_handlers", "cache_option_enabled"],
+        compatibilityFlags = ["nodejs_compat", "service_binding_extra_handlers"],
       )
     ),
   ],

src/workerd/api/http.c++

@@ -113,21 +113,6 @@ void requireValidHeaderValue(kj::StringPtr value) {
   }
 }
 
-Request::CacheMode getCacheModeFromName(kj::StringPtr value) {
-  if (value == "no-store") return Request::CacheMode::NOSTORE;
-  if (value == "no-cache") return Request::CacheMode::NOCACHE;
-  JSG_FAIL_REQUIRE(TypeError, kj::str("Unsupported cache mode: ", value));
-}
-
-jsg::Optional<kj::StringPtr> getCacheModeName(Request::CacheMode mode) {
-  switch (mode) {
-    case (Request::CacheMode::NONE): return kj::none;
-    case (Request::CacheMode::NOCACHE): return "no-cache"_kj;
-    case (Request::CacheMode::NOSTORE): return "no-store"_kj;
-  }
-  KJ_UNREACHABLE;
-}
-
 }  // namespace
 
 Headers::Headers(jsg::Dict<jsg::ByteString, jsg::ByteString> dict)
@@ -913,13 +898,6 @@ jsg::Ref<Request> Request::coerce(
         : Request::constructor(js, kj::mv(input), kj::mv(init));
 }
 
-jsg::Optional<kj::StringPtr> Request::getCache(jsg::Lock& js) {
-  return getCacheModeName(cacheMode);
-}
-Request::CacheMode Request::getCacheMode() {
-  return cacheMode;
-}
-
 jsg::Ref<Request> Request::constructor(
     jsg::Lock& js,
     Request::Info input,
@@ -932,7 +910,6 @@ jsg::Ref<Request> Request::constructor(
   CfProperty cf;
   kj::Maybe<Body::ExtractedBody> body;
   Redirect redirect = Redirect::FOLLOW;
-  CacheMode cacheMode = CacheMode::NONE;
 
   KJ_SWITCH_ONEOF(input) {
     KJ_CASE_ONEOF(u, kj::String) {
@@ -998,7 +975,6 @@ jsg::Ref<Request> Request::constructor(
           body = Body::ExtractedBody((oldJsBody)->detach(js), oldRequest->getBodyBuffer(js));
         }
       }
-      cacheMode = oldRequest->getCacheMode();
       redirect = oldRequest->getRedirectEnum();
       fetcher = oldRequest->getFetcher();
       signal = oldRequest->getSignal();
@@ -1075,13 +1051,6 @@ jsg::Ref<Request> Request::constructor(
               "response status code).");
         }
 
-        KJ_IF_SOME(c, initDict.cache) {
-          JSG_REQUIRE(FeatureFlags::get(js).getCacheOptionEnabled(), TypeError, kj::str(
-            "Unsupported cache mode: ", c,
-            "\nYou may need to enable the cache_option_enabled compatability flag."));
-          cacheMode = getCacheModeFromName(c);
-        }
-
         if (initDict.method != kj::none || initDict.body != kj::none) {
           // We modified at least one of the method or the body. In this case, we enforce the
           // spec rule that GET/HEAD requests cannot have bodies. (On the other hand, if neither
@@ -1096,7 +1065,6 @@ jsg::Ref<Request> Request::constructor(
       KJ_CASE_ONEOF(otherRequest, jsg::Ref<Request>) {
         method = otherRequest->method;
         redirect = otherRequest->redirect;
-        cacheMode = otherRequest->cacheMode;
         fetcher = otherRequest->getFetcher();
         signal = otherRequest->getSignal();
         headers = jsg::alloc<Headers>(*otherRequest->headers);
@@ -1118,7 +1086,7 @@ jsg::Ref<Request> Request::constructor(
   // TODO(conform): If `init` has a keepalive flag, pass it to the Body constructor.
   return jsg::alloc<Request>(method, url, redirect,
                  KJ_ASSERT_NONNULL(kj::mv(headers)), kj::mv(fetcher), kj::mv(signal),
-                 kj::mv(cf), kj::mv(body), cacheMode);
+                 kj::mv(cf), kj::mv(body));
 }
 
 jsg::Ref<Request> Request::clone(jsg::Lock& js) {
@@ -1233,10 +1201,6 @@ void Request::serialize(
 
     .cf = cf.getRef(js),
 
-    .cache = getCacheModeName(cacheMode).map([](kj::StringPtr name) -> kj::String {
-      return kj::str(name);
-    }),
-
     // .mode is unimplemented
     // .credentials is unimplemented
     // .cache is unimplemented
@@ -1807,15 +1771,6 @@ jsg::Promise<jsg::Ref<Response>> fetchImplNoOutputLock(
   kj::HttpHeaders headers(ioContext.getHeaderTable());
   jsRequest->shallowCopyHeadersTo(headers);
 
-  // If the jsRequest has a CacheMode, we need to handle that here.
-  // Currently, the only cache mode we support is undefined, but we will soon support
-  // no-cache and no-store. These additional modes will be hidden behind an autogate.
-  if (jsRequest->getCacheMode() != Request::CacheMode::NONE) {
-    return js.rejectedPromise<jsg::Ref<Response>>(
-        js.typeError(kj::str("Unsupported cache mode: ",
-            KJ_ASSERT_NONNULL(jsRequest->getCache(js)))));
-  }
-
   kj::String url = uriEncodeControlChars(
       urlList.back().toString(kj::Url::HTTP_PROXY_REQUEST).asBytes());
 

src/workerd/api/http.h

@@ -694,10 +694,9 @@ struct RequestInitializerDict {
   jsg::WontImplement credentials;
 
   // In browsers this controls the local browser cache. For Cloudflare Workers it could control the
-  // Cloudflare edge cache. While the standard defines a number of values for this property, our
-  // implementation supports only three: undefined (identifying the default caching behavior that
-  // has been implemented by the runtime), "no-store", and "no-cache".
-  jsg::Optional<kj::String> cache;
+  // Cloudflare edge cache. Note that this setting is different from using the `Cache-Control`
+  // header since `Cache-Control` would be forwarded to the origin.
+  jsg::Unimplemented cache;
 
   // These control how the `Referer` and `Origin` headers are initialized by the browser.
   // Browser-side JavaScript is normally not permitted to set these headers, because servers
@@ -770,21 +769,13 @@ class Request: public Body {
   };
   static kj::Maybe<Redirect> tryParseRedirect(kj::StringPtr redirect);
 
-  enum class CacheMode {
-    // CacheMode::NONE is set when cache is undefined. It represents the dafault cache
-    // mode that workers has supported.
-    NONE,
-    NOSTORE,
-    NOCACHE,
-  };
-
   Request(kj::HttpMethod method, kj::StringPtr url, Redirect redirect,
           jsg::Ref<Headers> headers, kj::Maybe<jsg::Ref<Fetcher>> fetcher,
           kj::Maybe<jsg::Ref<AbortSignal>> signal, CfProperty&& cf,
-          kj::Maybe<Body::ExtractedBody> body, CacheMode cacheMode = CacheMode::NONE)
+          kj::Maybe<Body::ExtractedBody> body)
     : Body(kj::mv(body), *headers), method(method), url(kj::str(url)),
       redirect(redirect), headers(kj::mv(headers)), fetcher(kj::mv(fetcher)),
-      cacheMode(cacheMode), cf(kj::mv(cf)) {
+      cf(kj::mv(cf)) {
     KJ_IF_SOME(s, signal) {
       // If the AbortSignal will never abort, assigning it to thisSignal instead ensures
       // that the cancel machinery is not used but the request.signal accessor will still
@@ -879,8 +870,15 @@ class Request: public Body {
   // TODO(conform): Won't implement?
 
   // The cache mode determines how HTTP cache is used with the request.
-  jsg::Optional<kj::StringPtr> getCache(jsg::Lock& js);
-  CacheMode getCacheMode();
+  // We currently do not fully implement this. Currently we will explicitly
+  // throw in the Request constructor if the option is set. For the accessor
+  // we want it to always just return undefined while it is not implemented.
+  // The spec does not provide a value to indicate "unimplemented" and all
+  // of the other values would imply semantics we do not follow. In discussion
+  // with other implementers with the same issues, it was decided that
+  // simply returning undefined for these was the best option.
+  // jsg::JsValue getCache(jsg::Lock& js) { return js.v8Undefined(); }
+  // TODO(conform): Won't implement?
 
   // We do not implement integrity checking at all. However, the spec says that
   // the default value should be an empty string. When the Request object is
@@ -910,9 +908,9 @@ class Request: public Body {
       // JSG_READONLY_PROTOTYPE_PROPERTY(duplex, getDuplex);
       // JSG_READONLY_PROTOTYPE_PROPERTY(mode, getMode);
       // JSG_READONLY_PROTOTYPE_PROPERTY(credentials, getCredentials);
+      // JSG_READONLY_PROTOTYPE_PROPERTY(cache, getCache);
       JSG_READONLY_PROTOTYPE_PROPERTY(integrity, getIntegrity);
       JSG_READONLY_PROTOTYPE_PROPERTY(keepalive, getKeepalive);
-      JSG_READONLY_PROTOTYPE_PROPERTY(cache, getCache);
 
       JSG_TS_OVERRIDE(<CfHostMetadata = unknown, Cf = CfProperties<CfHostMetadata>> {
         constructor(input: RequestInfo<CfProperties>, init?: RequestInit<Cf>);
@@ -936,6 +934,7 @@ class Request: public Body {
       // JSG_READONLY_INSTANCE_PROPERTY(duplex, getDuplex);
       // JSG_READONLY_INSTANCE_PROPERTY(mode, getMode);
       // JSG_READONLY_INSTANCE_PROPERTY(credentials, getCredentials);
+      // JSG_READONLY_INSTANCE_PROPERTY(cache, getCache);
       JSG_READONLY_INSTANCE_PROPERTY(integrity, getIntegrity);
       JSG_READONLY_INSTANCE_PROPERTY(keepalive, getKeepalive);
 
@@ -973,8 +972,6 @@ class Request: public Body {
   kj::Maybe<jsg::Ref<Fetcher>> fetcher;
   kj::Maybe<jsg::Ref<AbortSignal>> signal;
 
-  CacheMode cacheMode = CacheMode::NONE;
-
   // The fetch spec definition of Request has a distinction between the "signal" (which is
   // an optional AbortSignal passed in with the options), and "this' signal", which is an
   // AbortSignal that is always available via the request.signal accessor. When signal is

src/workerd/io/compatibility-date.capnp

@@ -504,10 +504,4 @@ struct CompatibilityFlags @0x8f8c1b68151b6cef {
       $compatDisableFlag("legacy_module_registry")
       $experimental;
   # Enables of the new module registry implementation.
-
-  cacheOptionEnabled @53 :Bool
-    $compatEnableFlag("cache_option_enabled")
-    $compatDisableFlag("cache_option_disabled")
-    $experimental;
-  # Enables the use of no-cache and no-store headers from requests
 }