Don't break out of frames when frame missing (#863)

Don't break out of frames when frame missing

This changes the default behaviour when a frame response is missing its
expected `<turbo-frame>` element.

Previously, when the response was missing its frame, we would trigger a
`turbo:frame-missing` event, and then (provided that event wasn't
cancelled) perform a full page visit to the requested URL.

However there are cases where the full reload makes things worse:

- If the frame contents were non-critical, reloading the page can turn a
  minor bug into a major one.
- It can mask some bugs where frames were intend to explicitly navigate
  out of the frame (`target="_top"`), by incurring a second request that
  loads the page that makes it seem as if it's working corrects.
- It leaves the user at a URL that may never be capable of rendering a
  valid response (since that URL was only intended to serve a particular
  frame). That means refreshing the page is no help in getting back to a
  working state.
- It can lose other temporary state on a page, like form values.

With this change, we no longer perform the full page visit. Instead, we
handle a missing frame by doing two things:

- Write a short error message into the frame, so that the problem is
  visible on the page.
- Throw an exception, which should make the problem quite obvious in
  development, and which allows it to be easily gathered by exception
  monitoring tools in production.

We keep the `turbo:frame-missing` event exactly as before, so
applications can still hook in to perform alternative behaviour if they
want.

src/core/errors.ts

@@ -0,0 +1 @@
+export class TurboFrameMissingError extends Error {}

src/core/frames/frame_controller.ts

@@ -32,6 +32,7 @@ import { VisitOptions } from "../drive/visit"
 import { TurboBeforeFrameRenderEvent } from "../session"
 import { StreamMessage } from "../streams/stream_message"
 import { PageSnapshot } from "../drive/page_snapshot"
+import { TurboFrameMissingError } from "../errors"
 
 type VisitFallback = (location: Response | Locatable, options: Partial<VisitOptions>) => Promise<void>
 export type TurboFrameMissingEvent = CustomEvent<{ response: Response; visit: VisitFallback }>
@@ -181,15 +182,9 @@ export class FrameController
           session.frameLoaded(this.element)
           this.fetchResponseLoaded(fetchResponse)
         } else if (this.willHandleFrameMissingFromResponse(fetchResponse)) {
-          console.warn(
-            `A matching frame for #${this.element.id} was missing from the response, transforming into full-page Visit.`
-          )
-          this.visitResponse(fetchResponse.response)
+          this.handleFrameMissingFromResponse(fetchResponse)
         }
       }
-    } catch (error) {
-      console.error(error)
-      this.view.invalidate()
     } finally {
       this.fetchResponseLoaded = () => {}
     }
@@ -264,7 +259,6 @@ export class FrameController
   }
 
   async requestFailedWithResponse(request: FetchRequest, response: FetchResponse) {
-    console.error(response)
     await this.loadResponse(response)
     this.resolveVisitPromise()
   }
@@ -432,6 +426,16 @@ export class FrameController
     return !event.defaultPrevented
   }
 
+  private handleFrameMissingFromResponse(fetchResponse: FetchResponse) {
+    this.view.missing()
+    this.throwFrameMissingError(fetchResponse)
+  }
+
+  private throwFrameMissingError(fetchResponse: FetchResponse) {
+    const message = `The response (${fetchResponse.statusCode}) did not contain the expected <turbo-frame id="${this.element.id}">`
+    throw new TurboFrameMissingError(message)
+  }
+
   private async visitResponse(response: Response): Promise<void> {
     const wrapped = new FetchResponse(response)
     const responseHTML = await wrapped.responseHTML

src/core/frames/frame_view.ts

@@ -5,8 +5,8 @@ import { View, ViewRenderOptions } from "../view"
 export type FrameViewRenderOptions = ViewRenderOptions<FrameElement>
 
 export class FrameView extends View<FrameElement> {
-  invalidate() {
-    this.element.innerHTML = ""
+  missing() {
+    this.element.innerHTML = `<strong class="turbo-frame-error">Content missing</strong>`
   }
 
   get snapshot() {

src/tests/functional/frame_tests.ts

@@ -3,7 +3,6 @@ import { assert, Assertion } from "chai"
 import {
   attributeForSelector,
   hasSelector,
-  innerHTMLForSelector,
   listenForEventOnTarget,
   nextAttributeMutationNamed,
   noNextAttributeMutationNamed,
@@ -142,47 +141,50 @@ test("test submitting a form[data-turbo-frame=_top] does not toggle the frame's
   )
 })
 
-test("test successfully following a link to a page without a matching frame dispatches a turbo:frame-missing event", async ({
+test("successfully following a link to a page without a matching frame dispatches a turbo:frame-missing event", async ({
   page,
 }) => {
   await page.click("#missing-frame-link")
-  await nextEventOnTarget(page, "missing", "turbo:before-fetch-request")
   const { response } = await nextEventOnTarget(page, "missing", "turbo:frame-missing")
 
-  assert.ok(await noNextEventNamed(page, "turbo:before-fetch-request"))
+  assert.equal(200, response.status)
+})
 
-  await nextEventNamed(page, "turbo:load")
+test("successfully following a link to a page without a matching frame shows an error and throws an exception", async ({
+  page,
+}) => {
+  let error: Error | undefined = undefined
+  page.once("pageerror", (e) => (error = e))
 
-  assert.ok(response, "dispatches turbo:frame-missing with event.detail.response")
-  assert.equal(pathname(page.url()), "/src/tests/fixtures/frames/frame.html", "navigates the page")
+  await page.click("#missing-frame-link")
 
-  await page.goBack()
-  await nextEventNamed(page, "turbo:load")
+  assert.match(await page.innerText("#missing"), /Content missing/)
 
-  assert.equal(pathname(page.url()), "/src/tests/fixtures/frames.html")
-  assert.ok(await innerHTMLForSelector(page, "#missing"))
+  assert.exists(error)
+  assert.equal(error!.message, `The response (200) did not contain the expected <turbo-frame id="missing">`)
 })
 
-test("test failing to follow a link to a page without a matching frame dispatches a turbo:frame-missing event", async ({
+test("failing to follow a link to a page without a matching frame dispatches a turbo:frame-missing event", async ({
   page,
 }) => {
   await page.click("#missing-page-link")
-  await nextEventOnTarget(page, "missing", "turbo:before-fetch-request")
   const { response } = await nextEventOnTarget(page, "missing", "turbo:frame-missing")
 
-  assert.ok(await noNextEventNamed(page, "turbo:before-fetch-request"))
-  assert.ok(await noNextEventNamed(page, "turbo:load"))
+  assert.equal(404, response.status)
+})
 
-  await nextEventNamed(page, "turbo:render")
+test("failing to follow a link to a page without a matching frame shows an error and throws an exception", async ({
+  page,
+}) => {
+  let error: Error | undefined = undefined
+  page.once("pageerror", (e) => (error = e))
 
-  assert.ok(response, "dispatches turbo:frame-missing with event.detail.response")
-  assert.equal(pathname(page.url()), "/missing.html", "navigates the page")
+  await page.click("#missing-page-link")
 
-  await page.goBack()
-  await nextEventNamed(page, "turbo:load")
+  assert.match(await page.innerText("#missing"), /Content missing/)
 
-  assert.equal(pathname(page.url()), "/src/tests/fixtures/frames.html")
-  assert.ok(await innerHTMLForSelector(page, "#missing"))
+  assert.exists(error)
+  assert.equal(error!.message, `The response (404) did not contain the expected <turbo-frame id="missing">`)
 })
 
 test("test the turbo:frame-missing event following a link to a page without a matching frame can be handled", async ({