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

Prevent a complete crash when DemoRecordingHelper fails #40

Merged
merged 2 commits into from
Jan 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ const waitForUserInputThenExit = () => {
subscriberManager.subscribe(new ShowHelpMessageWhenAsked());
subscriberManager.subscribe(new DemoRecordingHelper());
subscriberManager.subscribe(new DemoPlaybackHelper());
await subscriberManager.init()
subscriberManager.begin().then();
await subscriberManager.init();
await subscriberManager.begin();
})().catch(reason => {
log.fatal(reason);
log.fatal('Exited due to a fatal error. Please see above for details.');
Expand Down
4 changes: 3 additions & 1 deletion src/services/DemoRecordingHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ export class DemoRecordingHelper implements ListenerService {
await this.handleStartRecord();
} catch (e) {
//It's okay to throw errors in this method because it's an expectation that SubscriberManager knows what to do.
throw e;
// throw e;
DemoRecordingHelper.log.error(e);
SubscriberManagerFactory.getSubscriberManager().sendMessage('echo Failed to start recording. Check the log file for details.');
}
// The only other possible condition is DemoRecordingHelper.demoRecordingEndRegExp.test(consoleLine) being true.
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/utils/DemoNamingHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export class DemoNamingHelper {
}

public static getMapName = async (excludePrefix: boolean): Promise<string> => {
const mapLine = await SubscriberManagerFactory.getSubscriberManager().searchForValue('status', DemoNamingHelper.mapFromStatusRegExp, false);
const mapLine = await SubscriberManagerFactory.getSubscriberManager().searchForValue('status', DemoNamingHelper.mapFromStatusRegExp, false, 'to get the current map the player is on');
const mapName = DemoNamingHelper.mapFromStatusRegExp.exec(mapLine);
if (excludePrefix) {
//Attempt to remove the first underscore in the map name and everything before it
Expand Down
4 changes: 2 additions & 2 deletions src/utils/SubscriberManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,12 +209,12 @@ export class SubscriberManager {
return new TimeoutPromise().timeoutPromise(deferred.promise, `Request for Cvar '${cvarName}'`, false);
}

public searchForValue = (command: string | string[], regex: RegExp, isUserDecision: boolean) => {
public searchForValue = (command: string | string[], regex: RegExp, isUserDecision: boolean, additionalDetailsAboutRequest?: string) => {
const deferred: pDefer.DeferredPromise<string> = pDefer();
this.specialOutputGrabbers.push([regex, deferred]);
this.sendMessage(command);
this.valueListenersLog.debug(`Added a value grabber grabbing output from '${command}' to the grabber list.`);
return new TimeoutPromise().timeoutPromise(deferred.promise, `Request for response to command '${command}'`, isUserDecision);
return new TimeoutPromise().timeoutPromise(deferred.promise, `Request for response to command '${command}'`, isUserDecision, additionalDetailsAboutRequest);
}

public subscribe = (listener: ListenerService) => {
Expand Down
7 changes: 5 additions & 2 deletions src/utils/TimeoutPromise.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,20 @@ export class TimeoutPromise {
* @param promise a promise to add a timeout to
* @param descriptiveTaskName some name that will make sense to someone when they're trying to figure out the specific promise that failed and what it was doing
* @param isUserDecision whether or not this promise is for a user's decision, in which case the promise should wait console_user_input_wait_time seconds (per config.ini)
* @param additionalDetailsAboutRequest provides additional context for why this request was made. For example, 'to get the name of the current map'
* @returns a promise that will be rejected after config.internals.console_output_promise_wait_time milliseconds but won't be rejected if the provided promise resolves or is rejected before then
*/
public timeoutPromise = <T>(promise: Promise<T>, descriptiveTaskName: string, isUserDecision: boolean): Promise<T> => {
public timeoutPromise = <T>(promise: Promise<T>, descriptiveTaskName: string, isUserDecision: boolean, additionalDetailsAboutRequest?: string): Promise<T> => {
// Create the error here to not mess up the stacktrace
let err = Error(`${descriptiveTaskName} timed out in ${this.consolePromiseTimeoutMs}ms.${additionalDetailsAboutRequest ? ` The purpose of this request was ${additionalDetailsAboutRequest}.` : ''}`);
// Create a promise that rejects in <ms> milliseconds
let timeout = new Promise<T>((resolve, reject) => {
let id = setTimeout(() => {
clearTimeout(id);
if (isUserDecision) {
reject(new UserDecisionTimeoutException(descriptiveTaskName, this.consoleUserInputPromiseTimeoutMs));
} else {
reject(Error(`${descriptiveTaskName} timed out in ${this.consolePromiseTimeoutMs}ms.`));
reject(err);
}
}, isUserDecision ? this.consoleUserInputPromiseTimeoutMs : this.consolePromiseTimeoutMs);
})
Expand Down