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

(JS) HTTP requests should resolve on non-ok status codes #2046

Closed
Beanow opened this issue Jun 22, 2021 · 3 comments
Closed

(JS) HTTP requests should resolve on non-ok status codes #2046

Beanow opened this issue Jun 22, 2021 · 3 comments

Comments

@Beanow
Copy link
Member

Beanow commented Jun 22, 2021

Describe the bug

Currently non-ok status codes (like 404) will reject the requests promise, with an error like:
failed to execute API: Network Error: Status code 404 Not Found indicates failure (See #2004)

OK requests do resolve, including a status code:

/** Response object. */
interface Response<T> {
/** The request URL. */
url: string
/** The response status code. */
status: number
/** The response headers. */
headers: Record<string, string>
/** The response data. */
data: T
}

To Reproduce

  1. Using a snippet like this:
import { http } from "@tauri-apps/api";

// ...

// Using default client.
http
  .fetch("https://tauri.studio/nobody-home")
  .catch((reason) => console.error("REQUEST FAILED", reason));
  1. The inspector console will output an error such as:
[Error] REQUEST FAILED – "failed to execute API: Network Error: Status code 404 Not Found indicates failure"
	promiseReactionJob

Expected behavior

Any kind of status code should resolve, and the user should check whether the status code is an OK one.
For example using a helper like https://developer.mozilla.org/en-US/docs/Web/API/Response/ok

Rejection should be used when the request couldn't be completed, for example:

  • Timeouts
  • Name resolution errors
  • Being given invalid requests

Screenshots
n/a

Platform and Versions (please complete the following information):

tauri = { git = "https://github.com/tauri-apps/tauri", branch = "dev", features = ["api-all", "system-tray"] }

Operating System - Ubuntu, version 20.04 X64

Node.js environment
  Node.js - 16.1.0
  @tauri-apps/cli - 1.0.0-beta.1
  @tauri-apps/api - 1.0.0-beta.1

Global packages
  npm - 7.11.2
  yarn - 1.22.10

Rust environment
  rustc - 1.51.0
  cargo - 1.51.0

App directory structure
/src-tauri
/src
/node_modules
/build

App
  tauri.rs - 1.0.0-beta.1
build-type - bundle
CSP - default-src blob: data: filesystem: ws: http: https: 'unsafe-eval' 'unsafe-inline' 'self' img-src: 'self'
distDir - ../build
devPath - http://localhost:8080
framework - React

Additional context

Important information can be contained in non-ok server responses that should be available to the client.
For example response headers could contain rate limiting information, or the body could contain form validation errors.

@lucasfernog
Copy link
Member

Yeah I did see this mistake recently, but it would be a breaking change :|

@Beanow
Copy link
Member Author

Beanow commented Jun 22, 2021

Yeah, it is unfortunate to break APIs but shipping as-is will guarantee people having issues implementing what they need.
As an example: https://discord.com/developers/docs/topics/rate-limits

< HTTP/1.1 429 TOO MANY REQUESTS
< Content-Type: application/json
< Retry-After: 65
< X-RateLimit-Limit: 10
< X-RateLimit-Remaining: 0
< X-RateLimit-Reset: 1470173023.123
< X-RateLimit-Reset-After: 64.57
< X-RateLimit-Bucket: abcd1234
{
  "message": "You are being rate limited.",
  "retry_after": 64.57,
  "global": false
}

Contains tons of information to avoid + efficiently respond to rate-limiting.
But a 429 status would reject so the client wouldn't have access to it.

@lucasfernog
Copy link
Member

Since the HTTP API already has the status code on the resolved response, this can be seen as a feature or bug fix instead of a breaking change :)

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

No branches or pull requests

2 participants