-
Notifications
You must be signed in to change notification settings - Fork 432
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.
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
Raise turbo:error event on error #560
base: main
Are you sure you want to change the base?
Conversation
@@ -82,3 +84,8 @@ export function clearBusyState(...elements: Element[]) { | |||
element.removeAttribute("aria-busy") | |||
} | |||
} | |||
|
|||
export function reportError(error: FetchResponse | unknown) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want a better name for this as reportError
is an existing global: https://developer.mozilla.org/en-US/docs/Web/API/reportError
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can go with processError
instead?
Needs a rebase. |
Thanks for resurrecting this @salomvary! I was just looking for this functionality today 😄 |
This pr continues abandoned work on #357
I am struggling a bit with adding event firing tests as test cases for failure-scenario are missing for most of the affected functionality and I have not found any documentation on he expected behavior either. For example what do we expect if a request in a Turbo frame fails? Seems like the frame is not updated at all. What do we expect if a Drive link fails? Seems like the page gets updated with the error response.
There is still a test failing and I'm not entirely sure what to expect here:
And lastly, I'm not entirely sure what the semantics of
turbo:error
should be. Is this for any fetch error response or exception? Should it be limited to fetch errors only? If that's the case, wouldturbo:fetch-error
be a more appropriate name? I'm personally only interested in fetch error but #357 contains more.Anyway, feedback welcome.