Skip to content

Commit

Permalink
Avoid trueTime.now() throwing errors (#110)
Browse files Browse the repository at this point in the history
* Avoid trueTime.now() throwing errors

As it gets tricky to work with, this proposes to always return a time, even if that can be off initially.

Also makes the module try to synchronize itself as early as possible, and tries to avoid double sync network calls.
  • Loading branch information
osmestad authored Apr 16, 2024
1 parent 29ac6e0 commit dcaa1e5
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 46 deletions.
2 changes: 1 addition & 1 deletion eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export default [
'no-restricted-syntax': [
'error',
{
message: 'Never use Date.now! always use TrueTime',
message: 'Never use Date.now! always use trueTime.now()',
selector:
'CallExpression[callee.object.name="Date"][callee.property.name="now"]',
},
Expand Down
4 changes: 1 addition & 3 deletions packages/player/src/internal/true-time.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
import { TrueTime } from '@tidal-music/true-time';

export const trueTime = new TrueTime('https://api.tidal.com/v1/ping');
export { trueTime } from '@tidal-music/true-time';
28 changes: 1 addition & 27 deletions packages/true-time/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,36 +66,10 @@ describe('TrueTime', () => {
});
});

describe('true time errors', () => {
const trueTime = new TrueTime('https://api.tidal.com/v1/ping');

describe('now', () => {
test('throws error due to no synchronize call', async () => {
expect(() => trueTime.now()).toThrowError(
'Initialization has not been done yet. You need to call and await the synchronize method once.',
);
});
});

describe('timestamp', () => {
test('throws error due to no synchronize call', async () => {
// PS `performance.mark` should not be called without `startTime: trueTime.now()` option,
// but doing it here to test the error.
performance.mark('birds are dinosaurs');

expect(() => trueTime.timestamp('birds are dinosaurs')).toThrowError(
'Initialization has not been done yet. You need to call and await the synchronize method once.',
);
});
});
});

describe('google true time', () => {
const trueTime = new TrueTime('https://time.google.com');

test('fetches server time from correct url', async () => {
vi.spyOn(globalThis, 'fetch');
await trueTime.synchronize();
new TrueTime('https://time.google.com');

const callArg = vi.mocked(fetch)?.mock.calls[0]?.[0] as URL;
expect(callArg?.href).toEqual('https://time.google.com/');
Expand Down
28 changes: 13 additions & 15 deletions packages/true-time/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,28 @@
export class TrueTime {
#clientStartTime?: number;

#isSynchronizing = false;

#serverTime?: number;

#url: URL;

constructor(url: string) {
this.#url = new URL(url);
void this.synchronize();
}

/**
* Returns the current time adjusted to server-time.
*
* @param clientCurrentTime The current time on the client side. Defaults to Date.now().
* @returns The current adjusted time.
* @throws {ReferenceError} If the initialization has not been done yet. You need to call and await the `synchronize` method once.
* @returns The current adjusted time (or the client time if not synced yet).
*/
// eslint-disable-next-line no-restricted-syntax
now(clientCurrentTime = Date.now()): number {
if (!this.#serverTime || !this.#clientStartTime) {
throw new ReferenceError(
'Initialization has not been done yet. You need to call and await the synchronize method once.',
);
console.warn('TrueTime is not yet synchronized');
return clientCurrentTime;
}

return this.#serverTime + (clientCurrentTime - this.#clientStartTime);
Expand All @@ -37,13 +38,15 @@ export class TrueTime {
const anHour = 3_600_000;

if (
this.#clientStartTime &&
// eslint-disable-next-line no-restricted-syntax
Math.abs(Date.now() - this.#clientStartTime) < anHour
(this.#clientStartTime &&
// eslint-disable-next-line no-restricted-syntax
Math.abs(Date.now() - this.#clientStartTime) < anHour) ||
this.#isSynchronizing
) {
return;
}

this.#isSynchronizing = true;
try {
const response = await fetch(this.#url);

Expand All @@ -55,6 +58,7 @@ export class TrueTime {
} catch (error) {
console.error(error);
}
this.#isSynchronizing = false;
}

/**
Expand All @@ -64,17 +68,11 @@ export class TrueTime {
* @param markName - The name of the performance mark.
* @param detail - Optional. The detail of the performance mark.
* @returns The timestamp of the performance mark, or undefined if not found.
* @throws ReferenceError if initialization has not been done yet or if the performance mark is not found.
* @throws ReferenceError if the performance mark is not found.
*/
timestamp(markName: string, detail?: string): number | undefined {
let performanceEntry: PerformanceEntry | undefined;

if (!this.#serverTime || !this.#clientStartTime) {
throw new ReferenceError(
'Initialization has not been done yet. You need to call and await the synchronize method once.',
);
}

if (detail) {
performanceEntry = performance
.getEntriesByName(markName)
Expand Down

0 comments on commit dcaa1e5

Please sign in to comment.