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

Get debugger port from device logs #3540

Merged
merged 7 commits into from
May 23, 2018
Merged

Get debugger port from device logs #3540

merged 7 commits into from
May 23, 2018

Conversation

Fatme
Copy link
Contributor

@Fatme Fatme commented Apr 20, 2018

What is the current behavior?

The inspector listening socket is being closed 30 sec. after the last activity on it (opening, disconnection, etc). It had to be closed in order to have the hardcoded port number 18181
available for other apps after finishing (or never starting) a
debugging session. Without this timeout the first app that opened
the port would take it as long as it's running and would prevent
any subsequent debugging attempts of another app on the same iOS device;
or even the same app on another iOS Simulator on the same Mac machine.
This leads to unavailability to run fast sync on multiples simulators.

What is the new behavior?

From now on, the port is printed from ios-runtime in devices logs. {N} CLI parses the logs and searches for specified string. The string should be in following format:

NativeScript debugger has opened inspector socket on port ${port} for ${appId}.

This will allow to connect the frontend at any moment after starting a debug session, instead of failing after the 30 seconds have elapsed.

Should be merged after these:
telerik/mobile-cli-lib#1087
NativeScript/ios-sim-portable#103

Relying on this:
NativeScript/ios-jsc#907

Implementing:
#3321

@SvetoslavTsenov
Copy link

run ci

1 similar comment
@SvetoslavTsenov
Copy link

run ci

@dtopuzov
Copy link
Contributor

dtopuzov commented May 2, 2018

run ci

private attachToStartingApplicationEvent(device: Mobile.IDevice): void {
device.applicationManager.on(STARTING_IOS_APPLICATION_EVENT_NAME, (data: IIOSDebuggerPortData) => {
this.$logger.trace(STARTING_IOS_APPLICATION_EVENT_NAME, data);
const timer = setTimeout(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Consider if this is needed.

const factory = async () => {
const socket = device ? await device.connectToPort(inspectorBackendPort) : net.connect(inspectorBackendPort);
const port = await this.$iOSDebuggerPortService.getPort({ deviceId: debugData.deviceIdentifier, appId: debugData.applicationIdentifier });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thrown an error if port is null

@Fatme Fatme force-pushed the fatme/ios-debugger-port branch 3 times, most recently from 09fb48d to 9e5390d Compare May 11, 2018 13:39
@Fatme Fatme added the feature label May 14, 2018
@Fatme Fatme force-pushed the fatme/ios-debugger-port branch 3 times, most recently from 5b08ab7 to b1d750d Compare May 16, 2018 09:47
@Fatme
Copy link
Contributor Author

Fatme commented May 16, 2018

run ci

super();
}

public startLookingForDebuggerPort(device: Mobile.IDevice): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would use a more generic names for these methods, for example startParsingLog. The reason is that in the near future we may use the processDeviceLogResponse to detect when application is started and stopped on iOS. The method may have different regexes and emit different events, based on the information from the logs.
However, we can change this easily in the future, so this is not a merge stopper.

export class IOSDebuggerPortService implements IIOSDebuggerPortService {
private mapDebuggerPortData: IDictionary<IIOSDebuggerPortStoredData> = {};
private static DEFAULT_PORT = 18181;
private static MIN_REQUIRED_FRAMEWORK_VERSION = "4.0.0";
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be something like 4.1.0- (the current next maybe)

@rosen-vladimirov rosen-vladimirov added this to the 4.1.0 milestone May 18, 2018
@Fatme
Copy link
Contributor Author

Fatme commented May 21, 2018

run ci

@Natalia-Hristova
Copy link

run ci

@Fatme Fatme force-pushed the fatme/ios-debugger-port branch 2 times, most recently from 3b0a353 to c0c29b6 Compare May 22, 2018 08:55
@Fatme
Copy link
Contributor Author

Fatme commented May 22, 2018

run ci

@Fatme Fatme merged commit e0f81b6 into master May 23, 2018
@Fatme Fatme deleted the fatme/ios-debugger-port branch May 23, 2018 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants