From fd4741f08e6ccb68691a1c076e48e5e03f28443d Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Fri, 25 Sep 2026 09:23:16 -0400 Subject: [PATCH] fix: Fix player state related bugs * explicit shape for seeked return value eliminates some non-null assertions * guard against undefined variables --- .../PlayerState/AbstractPlayerState.ts | 56 ++++++++++++------- .../sources/PlayerState/ListenRange.ts | 13 +++-- .../PlayerState/PositionalPlayerState.ts | 16 ++++-- .../sources/PlayerState/RealtimePlayer.ts | 4 +- .../PlayerState/SubsonicPlayerState.ts | 3 +- 5 files changed, 59 insertions(+), 33 deletions(-) diff --git a/src/backend/sources/PlayerState/AbstractPlayerState.ts b/src/backend/sources/PlayerState/AbstractPlayerState.ts index ed5292dc..f467efad 100644 --- a/src/backend/sources/PlayerState/AbstractPlayerState.ts +++ b/src/backend/sources/PlayerState/AbstractPlayerState.ts @@ -217,7 +217,7 @@ export abstract class AbstractPlayerState { if (this.calculatedStatus !== CALCULATED_PLAYER_STATUSES.playing) { this.calculatedStatus = CALCULATED_PLAYER_STATUSES.unknown; } - return [this.getPlayedObject()!, played]; + return [this.requirePlayedObject(), played]; } else if (status !== undefined && !AbstractPlayerState.isProgressStatus(status)) { this.currentListenSessionEnd(); this.calculatedStatus = this.reportedStatus; @@ -235,12 +235,12 @@ export abstract class AbstractPlayerState { } this.logger.debug('New Play is a repeat'); this.setCurrentPlay(state, {reportedTS}); - return [this.getPlayedObject()!, played]; + return [this.requirePlayedObject(), played]; } else { if(this.currentListenRange !== undefined) { const [isSeeked, seekedPos] = this.currentListenRange.seeked(state.position, reportedTS); if (isSeeked !== false) { - this.logger.verbose(`Detected player was seeked ${(seekedPos! / 1000).toFixed(2)}s, starting new listen range`); + this.logger.verbose(`Detected player was seeked ${(seekedPos / 1000).toFixed(2)}s, starting new listen range`); if(state.position !== undefined && (this.currentListenRange as ListenRangePositional).end.position === state.position) { this.calculatedStatus = CALCULATED_PLAYER_STATUSES.paused; } @@ -255,8 +255,8 @@ export abstract class AbstractPlayerState { this.isRepeatPlay = false; // compensate for Players that report as STOPPED between Plays // -- should we check for closeToPlayStart() as well? - if(this.lastPlay !== undefined) { - const lastPlayDiff = Math.abs(this.lastPlayUpdatedAt!.diff(dayjs(), 's')); + if(this.lastPlay !== undefined && this.lastPlayUpdatedAt !== undefined) { + const lastPlayDiff = Math.abs(this.lastPlayUpdatedAt.diff(dayjs(), 's')); const shortDiff = lastPlayDiff < 20; const lastPlayMatch = playObjDataMatch(play, this.lastPlay); this.isRepeatPlay = shortDiff && lastPlayMatch; @@ -271,10 +271,10 @@ export abstract class AbstractPlayerState { this.reportedStatus = REPORTED_PLAYER_STATUSES.unknown; } - return [this.getPlayedObject()!, undefined]; + return [this.requirePlayedObject(), undefined]; } - protected incomingPlayMatchesExisting(play: PlayObject): boolean { return playObjDataMatch(this.currentPlay!, play); } + protected incomingPlayMatchesExisting(play: PlayObject): boolean { return this.currentPlay !== undefined && playObjDataMatch(this.currentPlay, play); } protected clearPlayer() { this.lastPlay = this.currentPlay; @@ -294,6 +294,15 @@ export abstract class AbstractPlayerState { this.currentListenSessionEnd(); } + /** Use when currentPlay is known to be set */ + protected requirePlayedObject(completed: boolean = false): PlayObject { + const played = this.getPlayedObject(completed); + if(played === undefined) { + throw new Error('Expected player to have a current Play but it does not'); + } + return played; + } + public getPlayedObject(completed: boolean = false): PlayObject | undefined { if(this.currentPlay !== undefined) { const ranges = [...this.listenRanges]; @@ -351,9 +360,14 @@ export abstract class AbstractPlayerState { * * Previous Position was close to end of Play */ protected isSessionRepeat(position?: number, reportedTS?: Dayjs): boolean { + const currentPlay = this.currentPlay; + // can't be close to start without a position + if(currentPlay === undefined || position === undefined) { + return false; + } if(this.currentListenRange !== undefined) { const [isSeeked, seekPos] = this.currentListenRange.seeked(position, reportedTS); - if (isSeeked === false || seekPos! > 0) { + if (isSeeked === false || seekPos > 0) { return false; } } @@ -361,15 +375,15 @@ export abstract class AbstractPlayerState { const hints: string[] = []; const repeatHint = `New Position (${position})`; - const trackDur = this.currentPlay!.data.duration; + const trackDur = currentPlay.data.duration; // new position is close to start of Play - const [closeStart, closeStartHint] = closeToPlayStart(this.currentPlay!, position!, {hintPrefix: false}); + const [closeStart, closeStartHint] = closeToPlayStart(currentPlay, position, {hintPrefix: false}); hints.push(closeStartHint); if (closeStart) { const playerDur = this.getListenDuration(); - const [repeatDurationOk, repeatDurationHint] = repeatDurationPlayed(this.currentPlay!, playerDur, {hintPrefix: false}); + const [repeatDurationOk, repeatDurationHint] = repeatDurationPlayed(currentPlay, playerDur, {hintPrefix: false}); // user has played at least 2 minutes or 50% of track // and the current (new) listen range was close to the start @@ -402,7 +416,7 @@ export abstract class AbstractPlayerState { } if (trackDur !== undefined && lastPosCandidate.length > 0) { for(const posData of lastPosCandidate) { - const [nearEnd, nearEndHint] = closeToPlayEnd(this.currentPlay!, posData.pos, {hintPrefix: false}); + const [nearEnd, nearEndHint] = closeToPlayEnd(currentPlay, posData.pos, {hintPrefix: false}); // last position is close to end of Play if(nearEnd) { this.logger.verbose(`${repeatHint} ${[closeStartHint, `${posData.context} (${posData.pos}s) ${nearEndHint}`].join(' and ')}`); @@ -443,24 +457,26 @@ export abstract class AbstractPlayerState { public textSummary() { const parts = ['']; let play: string; - if (this.currentPlay !== undefined) { - parts.push(`${buildTrackString(this.currentPlay, {include: ['trackId', 'artist', 'track', 'session']})} @ ${todayAwareFormat(this.playFirstSeenAt!)}`); + const currentPlay = this.currentPlay; + const duration = currentPlay?.data.duration; + if (currentPlay !== undefined) { + parts.push(`${buildTrackString(currentPlay, {include: ['trackId', 'artist', 'track', 'session']})} @ ${this.playFirstSeenAt !== undefined ? todayAwareFormat(this.playFirstSeenAt) : 'N/A'}`); } parts.push(`Reported: ${this.reportedStatus.toUpperCase()} | Calculated: ${this.calculatedStatus.toUpperCase()} | Stale: ${this.isUpdateStale() ? 'Yes' : 'No'} | Orphaned: ${this.isOrphaned() ? 'Yes' : 'No'} | Player Updated At: ${todayAwareFormat(this.stateLastUpdatedAt)} | Play Updated At: ${this.playLastUpdatedAt === undefined ? 'N/A' : todayAwareFormat(this.playLastUpdatedAt)}`); let progress = ''; - if (this.currentListenRange !== undefined && this.currentListenRange instanceof ListenRangePositional && this.currentPlay!.data.duration !== undefined && this.currentPlay!.data.duration !== 0) { - progress = `${progressBar(this.currentListenRange.end.position / this.currentPlay!.data.duration, 1, 15)} ${formatNumber(this.currentListenRange.end.position, {toFixed: 0})}/${formatNumber(this.currentPlay!.data.duration, {toFixed: 0})}s Reported | `; + if (this.currentListenRange !== undefined && this.currentListenRange instanceof ListenRangePositional && duration !== undefined && duration !== 0) { + progress = `${progressBar(this.currentListenRange.end.position / duration, 1, 15)} ${formatNumber(this.currentListenRange.end.position, {toFixed: 0})}/${formatNumber(duration, {toFixed: 0})}s Reported | `; } let listenedPercent = ''; - if (this.currentPlay !== undefined && this.currentPlay.data.duration !== undefined && this.currentPlay.data.duration !== 0) { - listenedPercent = formatNumber((this.getListenDuration() / this.currentPlay.data.duration) * 100, { + if (duration !== undefined && duration !== 0) { + listenedPercent = formatNumber((this.getListenDuration() / duration) * 100, { suffix: '%', toFixed: 0 }) } parts.push(`${progress}Listened For: ${formatNumber(this.getListenDuration(), {toFixed: 0})}s ${listenedPercent}`); - if (this.currentListenRange !== undefined && this.currentListenRange instanceof ListenRangePositional && this.currentListenRange.rtTruth) { - const rtProgress = `${progressBar((this.currentListenRange.rtPlayer.getPosition() / 1000) / this.currentPlay!.data.duration!, 1, 15)} ${formatNumber(this.currentListenRange.rtPlayer.getPosition() / 1000, {toFixed: 0})}/${formatNumber(this.currentPlay!.data.duration!, {toFixed: 0})}s`; + if (this.currentListenRange !== undefined && this.currentListenRange instanceof ListenRangePositional && this.currentListenRange.rtTruth && duration !== undefined) { + const rtProgress = `${progressBar((this.currentListenRange.rtPlayer.getPosition() / 1000) / duration, 1, 15)} ${formatNumber(this.currentListenRange.rtPlayer.getPosition() / 1000, {toFixed: 0})}/${formatNumber(duration, {toFixed: 0})}s`; parts.push(`${rtProgress} Realtime | Drifted ${formatNumber(Math.abs(this.currentListenRange.getDrift() / 1000), {toFixed: 1})}s (Max ${formatNumber(this.currentListenRange.getAllowedDrift() / 1000, {toFixed: 1})})`); } return parts.join('\n'); diff --git a/src/backend/sources/PlayerState/ListenRange.ts b/src/backend/sources/PlayerState/ListenRange.ts index 9bee3779..13e6bc3b 100644 --- a/src/backend/sources/PlayerState/ListenRange.ts +++ b/src/backend/sources/PlayerState/ListenRange.ts @@ -4,6 +4,9 @@ import { type ListenProgress, ListenProgressPositional, ListenProgressTS } from import type { RealtimePlayer } from "./RealtimePlayer.ts"; import { GenericRealtimePlayer } from "./RealtimePlayer.ts"; +/** [false] if not seeked, otherwise [true, seeked amount in ms] */ +export type SeekedResult = [false] | [true, Millisecond]; + export abstract class ListenRange { public start: ListenProgress; public end: ListenProgress; @@ -22,7 +25,7 @@ export abstract class ListenRange { public abstract isPositional(): boolean; public abstract isInitial(): boolean; - public abstract seeked(position?: number, reportedTS?: Dayjs): [boolean, Second?]; + public abstract seeked(position?: number, reportedTS?: Dayjs): SeekedResult; public abstract setRangeStart(data: ListenProgress | Partial): void; public abstract setRangeEnd(data: ListenProgress | Partial): void; public abstract getDuration(): Second; @@ -44,7 +47,7 @@ export class ListenRangeTS extends ListenRange implements ListenRangeData { return this.start.timestamp.isSame(this.end.timestamp); } - seeked(position?: number, reportedTS: Dayjs = dayjs()): [boolean, Second?] { + seeked(position?: number, reportedTS: Dayjs = dayjs()): SeekedResult { return [false]; } @@ -92,13 +95,13 @@ export class ListenRangePositional extends ListenRange { protected allowedDrift: number; - constructor(start?: ListenProgressPositional, end?: ListenProgressPositional, options: {rtTruth?: boolean, allowedDrift?: number, rtImmediate?: boolean} = {}) { + constructor(start: ListenProgressPositional, end?: ListenProgressPositional, options: {rtTruth?: boolean, allowedDrift?: number, rtImmediate?: boolean} = {}) { super(start, end); const { allowedDrift = 2000, rtTruth = false, rtImmediate = true } = options; this.allowedDrift = allowedDrift; this.rtTruth = rtTruth; this.rtPlayer = new GenericRealtimePlayer(); - this.rtPlayer.setPosition(start!.position * 1000); + this.rtPlayer.setPosition(start.position * 1000); if(rtImmediate) { this.rtPlayer.play(); } @@ -118,7 +121,7 @@ export class ListenRangePositional extends ListenRange { return this.start.position === this.end.position; } - seeked(position: Second, reportedTS: Dayjs = dayjs()): [boolean, Millisecond?] { + seeked(position: Second, reportedTS: Dayjs = dayjs()): SeekedResult { // if (new) position is earlier than last stored position then the user has seeked backwards on the player if (position < this.end.position) { return [true, (position - this.end.position) * 1000]; diff --git a/src/backend/sources/PlayerState/PositionalPlayerState.ts b/src/backend/sources/PlayerState/PositionalPlayerState.ts index 487f289c..f825a203 100644 --- a/src/backend/sources/PlayerState/PositionalPlayerState.ts +++ b/src/backend/sources/PlayerState/PositionalPlayerState.ts @@ -26,19 +26,23 @@ export class PositionalPlayerState extends AbstractPlayerState { this.rtTruth = rtTruth; } - protected newListenProgress(data?: Partial & Pick): ListenProgressPositional { - return new ListenProgressPositional(data!); + protected newListenProgress(data: Partial & Pick): ListenProgressPositional { + return new ListenProgressPositional(data); } - protected newListenRange(start?: ListenProgressPositional, end?: ListenProgressPositional, options: object = {}): ListenRangePositional { + protected newListenRange(start: ListenProgressPositional, end?: ListenProgressPositional, options: object = {}): ListenRangePositional { return new ListenRangePositional(start, end, {allowedDrift: this.allowedDrift, rtTruth: this.rtTruth, ...options}); } protected isSessionStillPlaying(position: number): boolean { //return this.reportedStatus === REPORTED_PLAYER_STATUSES.playing; - if(!this.currentListenRange!.isOverDrifted(position)) { + const range = this.currentListenRange; + if(range === undefined) { + return false; + } + if(!range.isOverDrifted(position)) { return true; } - return position !== this.currentListenRange!.end.position; + return position !== range.end.position; } protected currentListenSessionContinue(position: number = 0, timestamp?: Dayjs) { @@ -88,7 +92,7 @@ export class PositionalPlayerState extends AbstractPlayerState { data: { duration, } = {} - } = this.currentPlay!; + } = this.currentPlay ?? {}; if(duration !== undefined && duration !== 0 && (duration - this.currentListenRange.end.position) < this.gracefulEndBuffer) { // likely the track was listened to until it ended // but polling interval or network delays caused MS to not get data on the very end diff --git a/src/backend/sources/PlayerState/RealtimePlayer.ts b/src/backend/sources/PlayerState/RealtimePlayer.ts index 9e71ab53..b48ef91d 100644 --- a/src/backend/sources/PlayerState/RealtimePlayer.ts +++ b/src/backend/sources/PlayerState/RealtimePlayer.ts @@ -36,7 +36,9 @@ export abstract class RealtimePlayer { [Symbol.dispose]() { this.scheduler.stop(); for(const job of this.scheduler.getAllJobs()) { - this.scheduler.removeById(job.id!); + if(job.id !== undefined) { + this.scheduler.removeById(job.id); + } } } diff --git a/src/backend/sources/PlayerState/SubsonicPlayerState.ts b/src/backend/sources/PlayerState/SubsonicPlayerState.ts index bf21a0ea..91543ac6 100644 --- a/src/backend/sources/PlayerState/SubsonicPlayerState.ts +++ b/src/backend/sources/PlayerState/SubsonicPlayerState.ts @@ -58,7 +58,8 @@ export class SubsonicPlayerState extends PositionalPlayerState { } // if track has a duration and the listened duration for this session is greater than 100% + 5% (for buffer) // then assume track is on repeat - if(this.currentPlay!.data.duration !== undefined && this.getListenDuration() > (this.currentPlay!.data.duration + (0.05 * this.currentPlay!.data.duration))) { + const duration = this.currentPlay?.data.duration; + if(duration !== undefined && this.getListenDuration() > (duration + (0.05 * duration))) { this.logger.debug('Listened duration for this session is over 105%, triggering as a repeat'); return true; } -- 2.51.2