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

[ios] Time out packager liveness check after 10s #31367

Closed
wants to merge 1 commit into from

Conversation

radex
Copy link
Contributor

@radex radex commented Apr 15, 2021

Summary

isPackagerRunning check on iOS makes a http request to packager's /status endpoint to check if it's alive... The problem is if the packager can't be reached, but doesn't error out immediately. This can happen, for example, if running the app on device, and host computer's firewall doesn't allow a :8081 connection. In that case, the request will never succeed or fail until default timeout of 60s. It makes debugging the underlying issue quite unbearable. It's hard for me to imagine a legitimate packager connection that wouldn't respond in less than a second, so I propose a conservative timeout of 10s

Changelog

[iOS] [Fixed] - Don't hang app for 60s if packager can't be reached

Test Plan

Checked my app in dev mode to see if packager connects properly.

@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 Apr 15, 2021
@pull-bot
Copy link

Messages
📖

📋 Verify Changelog Format - A changelog entry has the following format: [CATEGORY] [TYPE] - Message.

CATEGORY may be:
  • General
  • iOS
  • Android
  • JavaScript
  • Internal (for changes that do not need to be called out in the release notes)

TYPE may be:

  • Added, for new features.
  • Changed, for changes in existing functionality.
  • Deprecated, for soon-to-be removed features.
  • Removed, for now removed features.
  • Fixed, for any bug fixes.
  • Security, in case of vulnerabilities.

MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.

Generated by 🚫 dangerJS against fe185d0

@analysis-bot
Copy link

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

Base commit: 8f3ffcf

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,853,157 +0
android hermes armeabi-v7a 8,374,661 +0
android hermes x86 9,310,080 +0
android hermes x86_64 9,252,987 +0
android jsc arm64-v8a 10,584,092 +0
android jsc armeabi-v7a 10,088,691 +0
android jsc x86 10,602,499 +0
android jsc x86_64 11,185,685 +0

Base commit: 8f3ffcf

@charlesbdudley charlesbdudley self-assigned this Sep 13, 2021
@facebook-github-bot
Copy link
Contributor

@charlesbdudley has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@charlesbdudley merged this pull request in c0e0446.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Sep 20, 2021
@dharit-tan
Copy link

dharit-tan commented Mar 7, 2023

Description

Posting this here in case it helps anyone - I ran into an issue where this change was causing the ios simulator to fail to connect to metro. Changing the timeout back to 10 seconds fixed the issue. From what we understand, the simulator was making a request to metro which was timing out, but we were unable to figure out why the request was taking so long.

macOs 13.1
iPhone 14 Pro simulator
XCode 14.2

Output of npx react-native info

warn Package
@sentry/react-native contains invalid configuration: "dependency.platforms.ios.sharedLibraries" is not allowed,"dependency.hooks" is not allowed. Please verify it's properly linked using "react-native config" command and contact the package maintainers about this.
warn Package rn-fetch-blob contains invalid configuration: "dependency.hooks" is not allowed. Please verify it's properly linked using "react-native config" command and contact the package maintainers about this.
info Fetching system and libraries information...
System:
OS: macOS 13.1
CPU: (10) arm64 Apple M1 Max
Memory: 546.97 MB / 32.00 GB
Shell: 5.8.1 - /bin/zsh
Binaries:
Node: 14.20.1 - ~/.nvm/versions/node/v14.20.1/bin/node
Yarn: 1.22.19 - ~/.yarn/bin/yarn
npm: 6.14.17 - ~/.nvm/versions/node/v14.20.1/bin/npm
Watchman: 2023.02.27.00 - /opt/homebrew/bin/watchman
Managers:
CocoaPods: 1.11.3 - /opt/homebrew/lib/ruby/gems/3.1.0/bin/pod
SDKs:
iOS SDK:
Platforms: DriverKit 22.2, iOS 16.2, macOS 13.1, tvOS 16.1, watchOS 9.1
Android SDK:
API Levels: 29, 30, 31, 33
Build Tools: 29.0.2, 30.0.2, 30.0.3, 31.0.0, 33.0.0
System Images: android-29 | Intel x86 Atom_64, android-29 | Google APIs Intel x86 Atom, android-33 | Google APIs ARM 64 v8a
Android NDK: Not Found
IDEs:
Android Studio: 2021.2 AI-212.5712.43.2112.8609683
Xcode: 14.2/14C18 - /usr/bin/xcodebuild
Languages:
Java: 11.0.11 - /usr/bin/javac
npmPackages:
@react-native-community/cli: Not Found
react: 18.2.0 => 18.2.0
react-native: 0.71.3 => 0.71.3
react-native-macos: Not Found
npmGlobalPackages:
react-native: Not Found

React Native Version

0.67.5 and above

Steps to reproduce

Hard to reproduce. Other people on my team were using the exact same setup, but I was the only one who encountered the issue.
For me it was simply using any react-native version that contains this commit, and running the app in iOS simulator.

Snack, code example, screenshot, or link to a repository

https://github.com/AudiusProject/audius-client

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants