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

bug: CapacitorHTTP fetch with Request object fails #6174

Closed
jn42lm1 opened this issue Dec 17, 2022 · 4 comments · Fixed by #6868 or #6908
Closed

bug: CapacitorHTTP fetch with Request object fails #6174

jn42lm1 opened this issue Dec 17, 2022 · 4 comments · Fixed by #6868 or #6908

Comments

@jn42lm1
Copy link

jn42lm1 commented Dec 17, 2022

Bug Report

Capacitor Version

💊   Capacitor Doctor  💊 

Latest Dependencies:

  @capacitor/cli: 4.6.1
  @capacitor/core: 4.6.1
  @capacitor/android: 4.6.1
  @capacitor/ios: 4.6.1

Installed Dependencies:

  @capacitor/cli: 4.6.1
  @capacitor/core: 4.6.1
  @capacitor/android: 4.6.1
  @capacitor/ios: 4.6.1

[success] iOS looking great! 👌
[success] Android looking great! 👌

Platform(s)

  • iOS
  • Android

Current Behavior

The Capacitor HTTP plugin supports a mode that patches the browser's window.fetch API when enabled.

With this active you can instead call the fetch API directly which will proxy all requests through the native Capacitor HTTP plugin on native platforms, i.e. Android, iOS. This is particularly useful to:

  1. not have to refactor standard fetch calls you might have in your codebase already
  2. enables support for observability frameworks, e.g. Instana, that also patch fetch to intercept HTTP traffic and that would not otherwise be able to detect calls to the Http.request() directly.

However, the current implementation of this patch in the Capacitor core/native-bridge.ts does not take into account that the browser's fetch API can be called with multiple argument signatures.

The patch expects all request to come in the form of fetch(resource: string, options: object) where resource is the URL string of the request, while options is the request options like a body, headers, etc.

The browser fetch API additionally allows for fetch requests to be made using a Request object instead, e.g. fetch(resource: Request). In this case the patch will fallback to using the browser fetch API instead of the Capacitor native HTTP plugin due to this limited implementation.

This is not the obvious behavior that a developer would expect. For example, making a request to a CORS protected resource with this 1st signature will work, while making a the request with the 2nd signature will fail.

To fix this, in core/NativeBridge.ts we can simply wrap the function arguments of the patch in a Request object to standardize further processing down stream across all possible function signatures.

Expected Behavior

With the fetch patch enabled, you should be able to call fetch using all browser supported signatures, e.g. including
fetch(resource: Request).

Code Reproduction

See this Demo Repo that shows the issue.

  • Run the demo on a native platform with either npm run android or npm run ios
    The demo contains 4 examples:
  1. a CORS request working with Capacitor.request
  2. a CORS fetch request working with fetch(resource: string, options: object)
  3. the same CORS fetch request failing with fetch(resource: Request)
  4. an example of how to fix the fetch patch in the NativeBridge.ts class to support all fetch function signatures

See this Merge Request Repo that fixes the issue.

Other Technical Details

npm --version output: 8.11.0

node --version output: v16.16.0

pod --version output (iOS issues only): 1.11.3

Additional Context

n/a

@jn42lm1
Copy link
Author

jn42lm1 commented Dec 17, 2022

Pull request #6175

@ionitron-bot
Copy link

ionitron-bot bot commented Jul 25, 2023

This issue has been labeled as type: bug. This label is added to issues that that have been reproduced and are being tracked in our internal issue tracker.

@suguruwataru
Copy link

Can reproduce...

I'm a newcomer to Capacitor, and this really has given me some headaches...

@ionitron-bot
Copy link

ionitron-bot bot commented Oct 14, 2023

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Capacitor, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Oct 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants