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

[Fix] Android/Fetch: Handle IOException when using fetch with HEAD method #30072

Closed
wants to merge 2 commits into from

Conversation

hank121314
Copy link
Contributor

@hank121314 hank121314 commented Sep 30, 2020

Summary

This pr fixes: #30055 .

Since fetch with HEAD method did not have body.
We should handle the error from Okhttp when it consuming the empty body.

Changelog

[Android] [Fixed] - HEAD requests with fetch should not be failed.

Test Plan

Please initiated a new project and replaced the app with the following code:

import React, { useEffect } from 'react';
import { StyleSheet, Text, View } from 'react-native';

export default function App() {
  useEffect(() => {
    fetch('https://determined-panini-990054.netlify.app/demo-image.png', {
      method: 'HEAD',
    })
      .then(console.warn)
      .catch(console.error);
  }, []);
  return (
    <View style={styles.container}>
      <Text>Open up App.js to start working on your app!</Text>
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    backgroundColor: '#fff',
    alignItems: 'center',
    justifyContent: 'center',
  },
});

It should console warn the result instead of Network request failed .

Thanks you so much for your code review!

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 30, 2020
@hank121314 hank121314 changed the title [Fix] Fetch: Handle IOException when using fetch with HEAD method [Fix] Android/Fetch: Handle IOException when using fetch with HEAD method Sep 30, 2020
@react-native-bot react-native-bot added Bug Platform: Android Android applications. labels Sep 30, 2020
@analysis-bot
Copy link

analysis-bot commented Sep 30, 2020

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,209,537 -297,651
android hermes armeabi-v7a 6,858,709 -240,255
android hermes x86 7,644,173 -301,162
android hermes x86_64 7,535,117 -321,274
android jsc arm64-v8a 9,369,162 397,259
android jsc armeabi-v7a 9,010,474 463,673
android jsc x86 9,231,894 260,360
android jsc x86_64 9,809,034 286,175

Base commit: 0045621

@analysis-bot
Copy link

analysis-bot commented Sep 30, 2020

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 0045621

public WritableMap toResponseData(Response response) throws IOException {
byte[] data = {};
try {
data = response.body().bytes();

Choose a reason for hiding this comment

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

What if we want to process a 204 no content? this would still fail when u call bytes on a null body

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed! Thank you for your suggestions 😄 !

@github-actions
Copy link

github-actions bot commented Aug 4, 2023

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Aug 4, 2023
@github-actions
Copy link

This PR was closed because it has been stalled for 7 days with no activity.

@github-actions github-actions bot closed this Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Platform: Android Android applications. Stale There has been a lack of activity on this issue and it may be closed soon.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HEAD requests fail on Android with 'TypeError: Network request failed'
5 participants