Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add turbo:fetch-request-error event on frame and form network errors #640

Merged
merged 17 commits into from
Aug 3, 2022
Merged

Add turbo:fetch-request-error event on frame and form network errors #640

merged 17 commits into from
Aug 3, 2022

Conversation

srt32
Copy link
Contributor

@srt32 srt32 commented Jul 20, 2022 •
edited
Loading

closes #462 and possible #460

This PR adds a new event called turbo:fetch-error that dispatches
when a form or frame fetch request errors.

This event will let developers respond appropriately when a fetch request
fails due network errors (such as on flaky wifi). A developer could reload the page or show an error message of some kind.

At the moment, if a frame navigation fetch request fails due to a network
error an error is thrown so

turbo/src/http/fetch_request.ts

Line 107 in 2d5cdda

return await this.receive(response)
never runs
and therefore the turbo:before-fetch-response event is not dispatched.

Here is a demo using http://127.0.0.1:9000/src/tests/fixtures/tabs.html as a fixture:

error.mp4

I'm very open to feedback or other ideas on a good direction to go here.

@srt32
Add turbo:fetch-error event on frame and form network errors
4828df4 
closes hotwired#462

This PR adds a new event called `turbo:fetch-error` that dispatches
when a form or frame fetch request errors.

This event will let developers respond appropriately when a fetch request
fails due network errors (such as on flaky wifi).

At the moment, if a frame navigation fetch request fails due to a network
error an error is thrown so https://github.com/hotwired/turbo/blob/2d5cdda4c030658da21965cb20d2885ca7c3e127/src/http/fetch_request.ts#L107 never runs
and therefore the `turbo:before-fetch-response` event is not dispatched.
keithamus
keithamus reviewed Jul 20, 2022
View reviewed changes
src/core/drive/form_submission.ts Outdated Show resolved Hide resolved
src/core/frames/frame_controller.ts Outdated Show resolved Hide resolved
srt32 and others added 2 commits July 20, 2022 05:54
@srt32 @keithamus
Update src/core/frames/frame_controller.ts
c6dfb4f 
Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>
@srt32 @keithamus
Update src/core/drive/form_submission.ts
ad0b408 
Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>
@seanpdoyle
Copy link
Contributor

Thank you for proposing this! Is it possible to add test coverage? Does playwright have the ability to force the Chrome or Firefox browser running behind the scenes into Offline mode?

@srt32
Copy link
Contributor Author

srt32 commented Jul 20, 2022

Thank you for proposing this! Is it possible to add test coverage? Does playwright have the ability to force the Chrome or Firefox browser running behind the scenes into Offline mode?

I've got something ugly in ecdd1c9 and curious if you know of a good way to determine which browser we need to pull in.

@srt32 srt32 changed the title Add turbo:fetch-error event on frame and form network errors Add turbo:fetch-request-error event on frame and form network errors Aug 2, 2022
@srt32
Copy link
Contributor Author

srt32 commented Aug 3, 2022

@seanpdoyle thanks for your help here! This change should be green and ready to go. I bumped into some recursion issues with the test helper so I opted to not serialize the detail for that specific event. I'm not sure exactly why this event is behaving differently than the others but the build is 🟢 now ;)

@srt32
Copy link
Contributor Author

srt32 commented Aug 3, 2022

Thanks for the thumb! What do we need to do to get this PR merged?

@seanpdoyle seanpdoyle mentioned this pull request Aug 16, 2022
Merged
@seanpdoyle
Copy link
Contributor

I've opened #685 to include <a> or Turbo.visit initiated fetch requests as well.

seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Aug 16, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
4ef8805 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Aug 16, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
030065c 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Aug 16, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
ed2748e 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Aug 16, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
23cdabc 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Aug 16, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
e6eb51b 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Aug 16, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
06d8a24 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Aug 16, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
bf1a833 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Aug 16, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
6eabca1 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Aug 16, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
35f352f 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Aug 16, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
541c54e 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Aug 16, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
66c7696 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Aug 16, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
133df88 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Aug 16, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
a598c2e 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Aug 16, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
2f96c26 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Aug 16, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
c253a6e 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Aug 16, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
72d0c06 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Aug 16, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
228559e 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Aug 17, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
cb34bf6 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Aug 17, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
731eb91 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Sep 13, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
873f355 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Sep 13, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
a51cfb2 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Sep 13, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
abc0a10 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Sep 14, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
c496c4d 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Sep 14, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
2516225 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Sep 14, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
283bc3b 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Sep 14, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
8c85542 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Sep 14, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits
e3be82f 
Follow-up to hotwired#640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
dhh pushed a commit that referenced this pull request Sep 14, 2022
@seanpdoyle
Dispatch turbo:fetch-request-error during Visits (#685)
2332d3d 
Follow-up to #640
Related to hotwired/turbo-site#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 `<turbo-frame>`- and `<form>`-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
`<turbo-frame>` and `<form>` error events.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keithamus keithamus keithamus left review comments

@seanpdoyle seanpdoyle seanpdoyle approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Provide event to handle turbo-frame hitting a NetworkError when fetching
4 participants
@srt32 @seanpdoyle @keithamus @dhh