From 133df884f5644986d0288ef44bb7ea7782e42614 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Tue, 16 Aug 2022 11:04:04 -0400 Subject: [PATCH] Dispatch `turbo:fetch-request-error` during Visits Follow-up to https://github.com/hotwired/turbo/pull/640 Related to https://github.com/hotwired/turbo-site/pull/110 When a `Visit` results in a `fetch` error (like when the browser is offline), dispatch a `turbo:fetch-request-error` event in the same style as ``- and `
`-initiated `fetch` errors. For the sake of consistency, also make the `TurboFetchRequestErrorEvent` cancelable. Along with the implementation change, this commit also adds test coverage to ensure that the `Event.target` is correct for `` and `` error events. --- src/core/drive/form_submission.ts | 5 ----- src/core/frames/frame_controller.ts | 6 +----- src/core/index.ts | 7 +++++-- src/core/session.ts | 2 -- src/http/fetch_request.ts | 18 +++++++++++++++++- src/tests/fixtures/form.html | 2 +- src/tests/functional/form_submission_tests.ts | 6 ++++++ src/tests/functional/frame_navigation_tests.ts | 2 +- src/tests/functional/visit_tests.ts | 10 ++++++++++ 9 files changed, 41 insertions(+), 17 deletions(-) diff --git a/src/core/drive/form_submission.ts b/src/core/drive/form_submission.ts index 2bed479b1..acc35b2ac 100644 --- a/src/core/drive/form_submission.ts +++ b/src/core/drive/form_submission.ts @@ -3,7 +3,6 @@ import { FetchResponse } from "../../http/fetch_response" import { expandURL } from "../url" import { dispatch, getAttribute, getMetaContent } from "../../util" import { StreamMessage } from "../streams/stream_message" -import { TurboFetchRequestErrorEvent } from "../session" export interface FormSubmissionDelegate { formSubmissionStarted(formSubmission: FormSubmission): void @@ -197,10 +196,6 @@ export class FormSubmission { requestErrored(request: FetchRequest, error: Error) { this.result = { success: false, error } - dispatch("turbo:fetch-request-error", { - target: this.formElement, - detail: { request, error }, - }) this.delegate.formSubmissionErrored(this, error) } diff --git a/src/core/frames/frame_controller.ts b/src/core/frames/frame_controller.ts index e276a8fe2..b3a363457 100644 --- a/src/core/frames/frame_controller.ts +++ b/src/core/frames/frame_controller.ts @@ -29,7 +29,7 @@ import { FrameRenderer } from "./frame_renderer" import { session } from "../index" import { isAction, Action } from "../types" import { VisitOptions } from "../drive/visit" -import { TurboBeforeFrameRenderEvent, TurboFetchRequestErrorEvent } from "../session" +import { TurboBeforeFrameRenderEvent } from "../session" import { StreamMessage } from "../streams/stream_message" export type TurboFrameMissingEvent = CustomEvent<{ fetchResponse: FetchResponse }> @@ -255,10 +255,6 @@ export class FrameController requestErrored(request: FetchRequest, error: Error) { console.error(error) - dispatch("turbo:fetch-request-error", { - target: this.element, - detail: { request, error }, - }) this.resolveVisitPromise() } diff --git a/src/core/index.ts b/src/core/index.ts index d33a64708..0b6ac51b1 100644 --- a/src/core/index.ts +++ b/src/core/index.ts @@ -19,7 +19,6 @@ export { TurboBeforeRenderEvent, TurboBeforeVisitEvent, TurboClickEvent, - TurboFetchRequestErrorEvent, TurboFrameLoadEvent, TurboFrameRenderEvent, TurboLoadEvent, @@ -29,7 +28,11 @@ export { export { TurboSubmitStartEvent, TurboSubmitEndEvent } from "./drive/form_submission" export { TurboFrameMissingEvent } from "./frames/frame_controller" -export { TurboBeforeFetchRequestEvent, TurboBeforeFetchResponseEvent } from "../http/fetch_request" +export { + TurboBeforeFetchRequestEvent, + TurboBeforeFetchResponseEvent, + TurboFetchRequestErrorEvent, +} from "../http/fetch_request" export { TurboBeforeStreamRenderEvent } from "../elements/stream_element" export { StreamActions } from "./streams/stream_actions" diff --git a/src/core/session.ts b/src/core/session.ts index 6df0c9b62..469e62bb1 100644 --- a/src/core/session.ts +++ b/src/core/session.ts @@ -21,7 +21,6 @@ import { FrameElement } from "../elements/frame_element" import { FrameViewRenderOptions } from "./frames/frame_view" import { FetchResponse } from "../http/fetch_response" import { Preloader, PreloaderDelegate } from "./drive/preloader" -import { FetchRequest } from "../http/fetch_request" export type FormMode = "on" | "off" | "optin" export type TimingData = unknown @@ -31,7 +30,6 @@ export type TurboBeforeVisitEvent = CustomEvent<{ url: string }> export type TurboClickEvent = CustomEvent<{ url: string; originalEvent: MouseEvent }> export type TurboFrameLoadEvent = CustomEvent export type TurboBeforeFrameRenderEvent = CustomEvent<{ newFrame: FrameElement } & FrameViewRenderOptions> -export type TurboFetchRequestErrorEvent = CustomEvent<{ request: FetchRequest; error: Error }> export type TurboFrameRenderEvent = CustomEvent<{ fetchResponse: FetchResponse }> export type TurboLoadEvent = CustomEvent<{ url: string; timing: TimingData }> export type TurboRenderEvent = CustomEvent diff --git a/src/http/fetch_request.ts b/src/http/fetch_request.ts index 459a9b786..983076469 100644 --- a/src/http/fetch_request.ts +++ b/src/http/fetch_request.ts @@ -10,6 +10,10 @@ export type TurboBeforeFetchRequestEvent = CustomEvent<{ export type TurboBeforeFetchResponseEvent = CustomEvent<{ fetchResponse: FetchResponse }> +export type TurboFetchRequestErrorEvent = CustomEvent<{ + request: FetchRequest + error: Error +}> export interface FetchRequestDelegate { referrer?: URL @@ -107,7 +111,9 @@ export class FetchRequest { return await this.receive(response) } catch (error) { if ((error as Error).name !== "AbortError") { - this.delegate.requestErrored(this, error as Error) + if (this.willDelegateErrorHandling(error as Error)) { + this.delegate.requestErrored(this, error as Error) + } throw error } } finally { @@ -175,4 +181,14 @@ export class FetchRequest { }) if (event.defaultPrevented) await requestInterception } + + private willDelegateErrorHandling(error: Error) { + const event = dispatch("turbo:fetch-request-error", { + target: this.target as EventTarget, + cancelable: true, + detail: { request: this, error: error }, + }) + + return !event.defaultPrevented + } } diff --git a/src/tests/fixtures/form.html b/src/tests/fixtures/form.html index 9cf0cfeca..fd19fed4f 100644 --- a/src/tests/fixtures/form.html +++ b/src/tests/fixtures/form.html @@ -127,7 +127,7 @@

Form

-
+
diff --git a/src/tests/functional/form_submission_tests.ts b/src/tests/functional/form_submission_tests.ts index 5bc9862ea..b175ae0b9 100644 --- a/src/tests/functional/form_submission_tests.ts +++ b/src/tests/functional/form_submission_tests.ts @@ -434,6 +434,12 @@ test("test invalid form submission with server error status", async ({ page }) = assert.notOk(await hasSelector(page, "#frame form.reject"), "replaces entire page") }) +test("test form submission with network error", async ({ page }) => { + await page.context().setOffline(true) + await page.click("#reject-form [type=submit]") + await nextEventOnTarget(page, "reject-form", "turbo:fetch-request-error") +}) + test("test submitter form submission reads button attributes", async ({ page }) => { const button = await page.locator("#submitter form button[type=submit][formmethod=post]") await button.click() diff --git a/src/tests/functional/frame_navigation_tests.ts b/src/tests/functional/frame_navigation_tests.ts index a38e27b85..b77469ec3 100644 --- a/src/tests/functional/frame_navigation_tests.ts +++ b/src/tests/functional/frame_navigation_tests.ts @@ -27,7 +27,7 @@ test("test frame navigation emits fetch-request-error event when offline", async await page.goto("/src/tests/fixtures/tabs.html") await page.context().setOffline(true) await page.click("#tab-2") - await nextEventNamed(page, "turbo:fetch-request-error") + await nextEventOnTarget(page, "tab-frame", "turbo:fetch-request-error") }) test("test promoted frame navigation updates the URL before rendering", async ({ page }) => { diff --git a/src/tests/functional/visit_tests.ts b/src/tests/functional/visit_tests.ts index 73ecb1fee..45ac5f950 100644 --- a/src/tests/functional/visit_tests.ts +++ b/src/tests/functional/visit_tests.ts @@ -7,6 +7,7 @@ import { isScrolledToTop, nextBeat, nextEventNamed, + nextEventOnTarget, noNextAttributeMutationNamed, readEventLogs, scrollToSelector, @@ -253,6 +254,15 @@ test("test can scroll to element after history-initiated turbo:visit", async ({ assert(await isScrolledToSelector(page, "#" + id), "scrolls after history-initiated turbo:load") }) +test("test Visit with network error", async ({ page }) => { + await page.evaluate(() => { + addEventListener("turbo:fetch-request-error", (event: Event) => event.preventDefault()) + }) + await page.context().setOffline(true) + await page.click("#same-origin-link") + await nextEventOnTarget(page, "html", "turbo:fetch-request-error") +}) + async function visitLocation(page: Page, location: string) { return page.evaluate((location) => window.Turbo.visit(location), location) }