Better error for opaque streams (#3001)

infra/testing/server/cross-origin-server.js

@@ -0,0 +1,46 @@
+/*
+  Copyright 2021 Google LLC
+
+  Use of this source code is governed by an MIT-style
+  license that can be found in the LICENSE file or at
+  https://opensource.org/licenses/MIT.
+*/
+
+const express = require('express');
+
+const PORT = 3010;
+
+let app;
+let server;
+
+function initApp(staticDir) {
+  app = express();
+  app.use(express.static(staticDir));
+}
+
+function start(staticDir) {
+  if (!app) {
+    initApp(staticDir);
+  }
+
+  return new Promise((resolve, reject) => {
+    server = app.listen(PORT, (error) => {
+      if (error) {
+        reject(error);
+      } else {
+        resolve(`http://localhost:${PORT}`);
+      }
+    });
+  });
+}
+
+function stop() {
+  if (server) {
+    server.close();
+  }
+}
+
+module.exports = {
+  start,
+  stop,
+};

packages/workbox-core/src/models/messages/messages.ts

@@ -366,4 +366,17 @@ export const messages: MessageMap = {
       `responses. It was passed a response with origin ${origin}.`
     );
   },
+
+  'opaque-streams-source': ({type}) => {
+    const message =
+      `One of the workbox-streams sources resulted in an ` +
+      `'${type}' response.`;
+    if (type === 'opaqueredirect') {
+      return (
+        `${message} Please do not use a navigation request that results ` +
+        `in a redirect as a source.`
+      );
+    }
+    return `${message} Please ensure your sources are CORS-enabled.`;
+  },
 };

packages/workbox-streams/src/concatenate.ts

@@ -6,10 +6,12 @@
   https://opensource.org/licenses/MIT.
 */
 
-import {logger} from 'workbox-core/_private/logger.js';
 import {assert} from 'workbox-core/_private/assert.js';
 import {Deferred} from 'workbox-core/_private/Deferred.js';
+import {logger} from 'workbox-core/_private/logger.js';
 import {StreamSource} from './_types.js';
+import {WorkboxError} from 'workbox-core/_private/WorkboxError.js';
+
 import './_version.js';
 
 /**
@@ -25,7 +27,11 @@ function _getReaderFromSource(
   source: StreamSource,
 ): ReadableStreamReader<unknown> {
   if (source instanceof Response) {
-    return source.body!.getReader();
+    // See https://github.com/GoogleChrome/workbox/issues/2998
+    if (source.body) {
+      return source.body.getReader();
+    }
+    throw new WorkboxError('opaque-streams-source', {type: source.type});
   }
   if (source instanceof ReadableStream) {
     return source.getReader();

test/workbox-streams/integration/test-all.js

@@ -7,8 +7,10 @@
 */
 
 const {expect} = require('chai');
-const activateAndControlSW = require('../../../infra/testing/activate-and-control');
 const {runUnitTests} = require('../../../infra/testing/webdriver/runUnitTests');
+const activateAndControlSW = require('../../../infra/testing/activate-and-control');
+const crossOriginServer = require('../../../infra/testing/server/cross-origin-server');
+const upath = require('upath');
 
 // Store local references of these globals.
 const {webdriver, server} = global.__workbox;
@@ -23,10 +25,19 @@ describe(`[workbox-streams] Integration Tests`, function () {
   const testServerAddress = server.getAddress();
   const testingURL = `${testServerAddress}/test/workbox-streams/static/`;
   const swURL = `${testingURL}sw.js`;
+  let crossOriginURL;
 
   before(async function () {
     await webdriver.get(testingURL);
     await activateAndControlSW(swURL);
+    const crossOrigin = await crossOriginServer.start(
+      upath.join('..', 'static'),
+    );
+    crossOriginURL = `${crossOrigin}/4.txt`;
+  });
+
+  after(function () {
+    crossOriginServer.stop();
   });
 
   for (const testCase of ['concatenate', 'concatenateToResponse', 'strategy']) {
@@ -58,4 +69,29 @@ describe(`[workbox-streams] Integration Tests`, function () {
       }
     });
   }
+
+  it(`should error when a stream source results in an opaque response`, async function () {
+    const {text} = await webdriver.executeAsyncScript(
+      async (crossOriginURL, cb) => {
+        try {
+          const url = new URL('/crossOriginURL', location);
+          url.searchParams.set('cross-origin-url', crossOriginURL);
+
+          const response = await fetch(url);
+          const text = await response.text();
+          cb({text});
+        } catch (error) {
+          cb({text: error.name});
+        }
+      },
+      crossOriginURL,
+    );
+
+    if (text === 'No streams support') {
+      this.skip();
+    } else {
+      // The exception name varies from browser to browser.
+      expect(text).to.be.oneOf(['TypeError', 'AbortError']);
+    }
+  });
 });

test/workbox-streams/static/sw.js

@@ -23,9 +23,26 @@ const getSourceFunctions = () => [
 ];
 
 self.addEventListener('fetch', (event) => {
+  const url = new URL(event.request.url);
+  const crossOriginURL = url.searchParams.get('cross-origin-url');
+
   if (!workbox.streams.isSupported()) {
     event.respondWith(new Response('No streams support'));
-  } else if (event.request.url.endsWith('concatenateToResponse')) {
+  } else if (crossOriginURL) {
+    const {done, response} = workbox.streams.concatenateToResponse(
+      [
+        () => 'this will',
+        () => fetch(crossOriginURL, {mode: 'no-cors'}),
+        () => 'error',
+      ].map((f) => f()),
+      {
+        'content-type': 'text/plain',
+        'x-test-case': 'crossOriginURL',
+      },
+    );
+    event.respondWith(response);
+    event.waitUntil(done);
+  } else if (url.pathname.endsWith('concatenateToResponse')) {
     const {done, response} = workbox.streams.concatenateToResponse(
       getSourceFunctions().map((f) => f()),
       {
@@ -35,7 +52,7 @@ self.addEventListener('fetch', (event) => {
     );
     event.respondWith(response);
     event.waitUntil(done);
-  } else if (event.request.url.endsWith('concatenate')) {
+  } else if (url.pathname.endsWith('concatenate')) {
     const {done, stream} = workbox.streams.concatenate(
       getSourceFunctions().map((f) => f()),
     );
@@ -51,7 +68,7 @@ self.addEventListener('fetch', (event) => {
 });
 
 workbox.routing.registerRoute(
-  new RegExp('strategy$'),
+  ({url}) => url.pathname.endsWith('strategy'),
   workbox.streams.strategy(getSourceFunctions(), {
     'content-type': 'text/plain',
     'x-test-case': 'strategy',