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.

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

refactor(core): return 200 on any IPC call, closes #10286 #10585

Merged
merged 3 commits into from
Aug 13, 2024

Conversation

lucasfernog
Copy link
Member

By default the webview prints a Failed to load resource: the server responded with a status of 400 (Bad Request) ipc://localhost error message when a command returns an error, which is confusing to users.

This changes the IPC to return status 200 on any call, with a header to indicate whether the result was ok or not. This removes the console error, which would only log the actual error result if it isn't caught by the user.

By default the webview prints a `Failed to load resource: the server responded with a status of 400 (Bad Request) ipc://localhost` error message when a command returns an error, which is confusing to users.

This changes the IPC to return status 200 on any call, with a header to indicate whether the result was ok or not. This removes the console error, which would only log the actual error result if it isn't caught by the user.
@lucasfernog lucasfernog requested a review from a team as a code owner August 12, 2024 23:20
Copy link
Contributor

github-actions bot commented Aug 12, 2024

Package Changes Through de7ce6a

There are 6 changes which include tauri-build with prepatch, tauri-codegen with prerelease, tauri-utils with prerelease, tauri-cli with prerelease, @tauri-apps/cli with prerelease, tauri with prerelease

Planned Package Versions

The following package releases are the planned based on the context of changes in this pull request.

package current next
tauri-utils 2.0.0-rc.2 2.0.0-rc.3
tauri-bundler 2.0.1-rc.1 2.0.1-rc.2
tauri-runtime 2.0.0-rc.2 2.0.0-rc.3
tauri-runtime-wry 2.0.0-rc.2 2.0.0-rc.3
tauri-codegen 2.0.0-rc.2 2.0.0-rc.3
tauri-macros 2.0.0-rc.2 2.0.0-rc.3
tauri-plugin 2.0.0-rc.2 2.0.0-rc.3
tauri-build 2.0.0-rc.2 2.0.1-rc.0
tauri 2.0.0-rc.2 2.0.0-rc.3
@tauri-apps/cli 2.0.0-rc.3 2.0.0-rc.4
tauri-cli 2.0.0-rc.3 2.0.0-rc.4

Add another change file through the GitHub UI by following this link.


Read about change files or the docs at github.com/jbolda/covector

amrbashir
amrbashir previously approved these changes Aug 13, 2024
Copy link
Member

@amrbashir amrbashir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we add a change file?

@@ -42,7 +42,8 @@
}
})
.then((response) => {
const cb = response.ok ? callback : error
const cb =
response.headers.get('tauri-response') === 'ok' ? callback : error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we use Tauri-Response to match the header on Rust side (just for consistency, it doesn't matter logically)

@Legend-Master
Copy link
Contributor

Legend-Master commented Aug 13, 2024

I honestly quite like it being a network error, yes, I got confused by it before as well, but after I understand what it means, it's quite handy to quickly debug the errors (without having to manually put break point/logging in to the code to have a rough idea of what happened), maybe consider document this instead of changing it to always return 200?

Never mind, devtools is powerful enough to display a response header in the network panel

image

@lucasfernog lucasfernog merged commit fedf93e into dev Aug 13, 2024
20 checks passed
@lucasfernog lucasfernog deleted the refactor/ipc-errors branch August 13, 2024 11:39
@amrbashir amrbashir mentioned this pull request Aug 16, 2024
@dygy
Copy link

dygy commented Aug 16, 2024

what i should update and to which version to receive a fix of 400 ?

@dygy
Copy link

dygy commented Aug 16, 2024

should wait until rc-3 publish?

@FabianLars
Copy link
Member

should wait until rc-3 publish?

yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants