ionic-team / capacitor

Build cross-platform Native Progressive Web Apps for iOS, Android, and the Web ⚡️
https://capacitorjs.com
MIT License
12.21k stars 1.01k forks source link

bug: CapacitorHTTP fetch with Request object fails #6174

Closed jn42lm1 closed 1 year ago

jn42lm1 commented 1 year ago

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)

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.

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 commented 1 year ago

Pull request https://github.com/ionic-team/capacitor/pull/6175

ionitron-bot[bot] commented 1 year ago

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 commented 1 year ago

Can reproduce...

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

ionitron-bot[bot] commented 1 year ago

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.