From cfa267bc0916ede09c8b14aedcdb69a791055fb6 Mon Sep 17 00:00:00 2001 From: Nicholas Jamieson Date: Sat, 31 Oct 2020 11:07:35 +1000 Subject: [PATCH] feat: stopped notification handler (#5750) * fix: add undefined to config Promise type And fix a spelling error. * feat: add stopped registration point * chore: wire up Subscriber * chore: make call async * chore: use notification exports * chore: check callback before setTimeout * chore: use null * chore: api_guardian:update * refactor: move factories to avoid circular dep * chore: api_guardian:update * chore: remove empty lines * chore: remove canReportError and add tests * test: use onStoppedNotification * chore: remove unused import * refactor: import notification factories directly * refactor: use delegatable provider * test: replace setTimeout with the provider * chore: add and use timeout provider * chore: use ObservableNotification type * chore: update api_guardian files * chore: comment the delegation of setTimeout, et al. --- api_guard/dist/types/index.d.ts | 3 +- spec/Observable-spec.ts | 9 +- spec/config-spec.ts | 169 ++++++++++++++++------ spec/operators/groupBy-spec.ts | 2 +- spec/schedulers/TestScheduler-spec.ts | 28 +++- spec/util/canReportError-spec.ts | 24 --- src/internal/Notification.ts | 39 ----- src/internal/NotificationFactories.ts | 41 ++++++ src/internal/Observable.ts | 23 +-- src/internal/Subscriber.ts | 26 +++- src/internal/config.ts | 18 ++- src/internal/scheduler/timeoutProvider.ts | 28 ++++ src/internal/testing/TestScheduler.ts | 58 +++++++- src/internal/util/reportUnhandledError.ts | 3 +- 14 files changed, 318 insertions(+), 153 deletions(-) delete mode 100644 spec/util/canReportError-spec.ts create mode 100644 src/internal/NotificationFactories.ts create mode 100644 src/internal/scheduler/timeoutProvider.ts diff --git a/api_guard/dist/types/index.d.ts b/api_guard/dist/types/index.d.ts index e69478ab60..7e9db4977c 100644 --- a/api_guard/dist/types/index.d.ts +++ b/api_guard/dist/types/index.d.ts @@ -112,7 +112,8 @@ export declare function concat(...inputsAndSchedul export declare const config: { onUnhandledError: ((err: any) => void) | null; - Promise: PromiseConstructorLike; + onStoppedNotification: ((notification: ObservableNotification, subscriber: Subscriber) => void) | null; + Promise: PromiseConstructorLike | undefined; useDeprecatedSynchronousErrorHandling: boolean; useDeprecatedNextContext: boolean; }; diff --git a/spec/Observable-spec.ts b/spec/Observable-spec.ts index 32c8fd31a4..9fc41f7ed7 100644 --- a/spec/Observable-spec.ts +++ b/spec/Observable-spec.ts @@ -965,12 +965,13 @@ describe('Observable.lift', () => { }); it('should not swallow internal errors', (done) => { - config.onUnhandledError = (err) => { - expect(err).to.equal('bad'); - config.onUnhandledError = null; + config.onStoppedNotification = (notification) => { + expect(notification.kind).to.equal('E'); + expect(notification).to.have.property('error', 'bad'); + config.onStoppedNotification = null; done(); }; - + new Observable(subscriber => { subscriber.error('test'); throw 'bad'; diff --git a/spec/config-spec.ts b/spec/config-spec.ts index 248bceae2d..0e0cf88e0c 100644 --- a/spec/config-spec.ts +++ b/spec/config-spec.ts @@ -3,6 +3,7 @@ import { config } from '../src/internal/config'; import { expect } from 'chai'; import { Observable } from 'rxjs'; +import { timeoutProvider } from 'rxjs/internal/scheduler/timeoutProvider'; describe('config', () => { it('should have a Promise property that defaults to nothing', () => { @@ -23,19 +24,19 @@ describe('config', () => { let called = false; const results: any[] = []; - config.onUnhandledError = err => { + config.onUnhandledError = (err) => { called = true; expect(err).to.equal('bad'); - done() + done(); }; - const source = new Observable(subscriber => { + const source = new Observable((subscriber) => { subscriber.next(1); subscriber.error('bad'); }); source.subscribe({ - next: value => results.push(value), + next: (value) => results.push(value), }); expect(called).to.be.false; expect(results).to.deep.equal([1]); @@ -45,31 +46,31 @@ describe('config', () => { let called = false; const results: any[] = []; - config.onUnhandledError = err => { + config.onUnhandledError = (err) => { called = true; expect(err).to.equal('bad'); - done() + done(); }; - const source = new Observable(subscriber => { + const source = new Observable((subscriber) => { subscriber.next(1); subscriber.error('bad'); }); - source.subscribe(value => results.push(value)); + source.subscribe((value) => results.push(value)); expect(called).to.be.false; expect(results).to.deep.equal([1]); }); it('should call asynchronously if an error is emitted and not handled by the consumer in the empty case', (done) => { let called = false; - config.onUnhandledError = err => { + config.onUnhandledError = (err) => { called = true; expect(err).to.equal('bad'); - done() + done(); }; - const source = new Observable(subscriber => { + const source = new Observable((subscriber) => { subscriber.error('bad'); }); @@ -77,41 +78,88 @@ describe('config', () => { expect(called).to.be.false; }); - it('should call asynchronously if a subscription setup errors after the subscription is closed by an error', (done) => { + /** + * This test is added so people know this behavior is _intentional_. It's part of the contract of observables + * and, while I'm not sure I like it, it might start surfacing untold numbers of errors, and break + * node applications if we suddenly changed this to start throwing errors on other jobs for instances + * where users accidentally called `subscriber.error` twice. Likewise, would we report an error + * for two calls of `complete`? This is really something a build-time tool like a linter should + * capture. Not a run time error reporting event. + */ + it('should not be called if two errors are sent to the subscriber', (done) => { let called = false; - config.onUnhandledError = err => { + config.onUnhandledError = () => { called = true; - expect(err).to.equal('bad'); - done() }; - const source = new Observable(subscriber => { + const source = new Observable((subscriber) => { + subscriber.error('handled'); + subscriber.error('swallowed'); + }); + + let syncSentError: any; + source.subscribe({ + error: (err) => { + syncSentError = err; + }, + }); + + expect(syncSentError).to.equal('handled'); + // When called, onUnhandledError is called on a timeout, so delay the + // the assertion of the expectation until after the point at which + // onUnhandledError would have been called. + timeoutProvider.setTimeout(() => { + expect(called).to.be.false; + done(); + }); + }); + }); + + describe('onStoppedNotification', () => { + afterEach(() => { + config.onStoppedNotification = null; + }); + + it('should default to null', () => { + expect(config.onStoppedNotification).to.be.null; + }); + + it('should be called asynchronously if a subscription setup errors after the subscription is closed by an error', (done) => { + let called = false; + config.onStoppedNotification = (notification) => { + called = true; + expect(notification.kind).to.equal('E'); + expect(notification).to.have.property('error', 'bad'); + done(); + }; + + const source = new Observable((subscriber) => { subscriber.error('handled'); throw 'bad'; }); let syncSentError: any; source.subscribe({ - error: err => { + error: (err) => { syncSentError = err; - } + }, }); expect(syncSentError).to.equal('handled'); expect(called).to.be.false; }); - it('should call asynchronously if a subscription setup errors after the subscription is closed by a completion', (done) => { + it('should be called asynchronously if a subscription setup errors after the subscription is closed by a completion', (done) => { let called = false; let completed = false; - - config.onUnhandledError = err => { + config.onStoppedNotification = (notification) => { called = true; - expect(err).to.equal('bad'); - done() + expect(notification.kind).to.equal('E'); + expect(notification).to.have.property('error', 'bad'); + done(); }; - const source = new Observable(subscriber => { + const source = new Observable((subscriber) => { subscriber.complete(); throw 'bad'; }); @@ -122,47 +170,80 @@ describe('config', () => { }, complete: () => { completed = true; - } + }, }); expect(completed).to.be.true; expect(called).to.be.false; }); - /** - * Thie test is added so people know this behavior is _intentional_. It's part of the contract of observables - * and, while I'm not sure I like it, it might start surfacing untold numbers of errors, and break - * node applications if we suddenly changed this to start throwing errors on other jobs for instances - * where users accidentally called `subscriber.error` twice. Likewise, would we report an error - * for two calls of `complete`? This is really something a build-time tool like a linter should - * capture. Not a run time error reporting event. - */ - it('should not be called if two errors are sent to the subscriber', (done) => { + it('should be called if a next is sent to the stopped subscriber', (done) => { let called = false; - config.onUnhandledError = () => { + config.onStoppedNotification = (notification) => { called = true; + expect(notification.kind).to.equal('N'); + expect(notification).to.have.property('value', 2); + done(); }; - const source = new Observable(subscriber => { + const source = new Observable((subscriber) => { + subscriber.next(1); + subscriber.complete(); + subscriber.next(2); + }); + + let syncSentValue: any; + source.subscribe({ + next: (value) => { + syncSentValue = value; + }, + }); + + expect(syncSentValue).to.equal(1); + expect(called).to.be.false; + }); + + it('should be called if two errors are sent to the subscriber', (done) => { + let called = false; + config.onStoppedNotification = (notification) => { + called = true; + expect(notification.kind).to.equal('E'); + expect(notification).to.have.property('error', 'swallowed'); + done(); + }; + + const source = new Observable((subscriber) => { subscriber.error('handled'); subscriber.error('swallowed'); }); let syncSentError: any; source.subscribe({ - error: err => { + error: (err) => { syncSentError = err; - } + }, }); expect(syncSentError).to.equal('handled'); - // This timeout would be scheduled _after_ any error timeout that might be scheduled - // (But we're not scheduling that), so this is just an artificial delay to make sure the - // behavior sticks. - setTimeout(() => { - expect(called).to.be.false; + expect(called).to.be.false; + }); + + it('should be called if two completes are sent to the subscriber', (done) => { + let called = false; + config.onStoppedNotification = (notification) => { + called = true; + expect(notification.kind).to.equal('C'); done(); + }; + + const source = new Observable((subscriber) => { + subscriber.complete(); + subscriber.complete(); }); + + source.subscribe(); + + expect(called).to.be.false; }); }); -}); \ No newline at end of file +}); diff --git a/spec/operators/groupBy-spec.ts b/spec/operators/groupBy-spec.ts index df671f32ad..6b31555c9a 100644 --- a/spec/operators/groupBy-spec.ts +++ b/spec/operators/groupBy-spec.ts @@ -3,7 +3,7 @@ import { groupBy, delay, tap, map, take, mergeMap, materialize, skip, ignoreElem import { TestScheduler } from 'rxjs/testing'; import { ReplaySubject, of, Observable, Operator, Observer, interval, Subject } from 'rxjs'; import { hot, cold, expectObservable, expectSubscriptions } from '../helpers/marble-testing'; -import { createNotification } from 'rxjs/internal/Notification'; +import { createNotification } from 'rxjs/internal/NotificationFactories'; declare const rxTestScheduler: TestScheduler; diff --git a/spec/schedulers/TestScheduler-spec.ts b/spec/schedulers/TestScheduler-spec.ts index 67c3c3e425..4873dfb12e 100644 --- a/spec/schedulers/TestScheduler-spec.ts +++ b/spec/schedulers/TestScheduler-spec.ts @@ -3,10 +3,11 @@ import { hot, cold, expectObservable, expectSubscriptions, time } from '../helpe import { TestScheduler } from 'rxjs/testing'; import { Observable, NEVER, EMPTY, Subject, of, merge, animationFrameScheduler, asapScheduler, asyncScheduler, interval } from 'rxjs'; import { delay, debounceTime, concatMap, mergeMap, mapTo, take } from 'rxjs/operators'; -import { nextNotification, COMPLETE_NOTIFICATION, errorNotification } from 'rxjs/internal/Notification'; +import { nextNotification, COMPLETE_NOTIFICATION, errorNotification } from 'rxjs/internal/NotificationFactories'; import { animationFrameProvider } from 'rxjs/internal/scheduler/animationFrameProvider'; import { immediateProvider } from 'rxjs/internal/scheduler/immediateProvider'; import { intervalProvider } from 'rxjs/internal/scheduler/intervalProvider'; +import { timeoutProvider } from 'rxjs/internal/scheduler/timeoutProvider'; declare const rxTestScheduler: TestScheduler; @@ -680,22 +681,41 @@ describe('TestScheduler', () => { }); }); - it('should schedule immediates before intervals', () => { + it('should schedule timeouts', () => { + const testScheduler = new TestScheduler(assertDeepEquals); + testScheduler.run(() => { + const values: string[] = []; + const { setTimeout } = timeoutProvider; + setTimeout(() => { + values.push(`a@${testScheduler.now()}`); + }, 1); + expect(values).to.deep.equal([]); + testScheduler.schedule(() => { + expect(values).to.deep.equal(['a@1']); + }, 10); + }); + }); + + it('should schedule immediates before intervals and timeouts', () => { const testScheduler = new TestScheduler(assertDeepEquals); testScheduler.run(() => { const values: string[] = []; const { setImmediate } = immediateProvider; const { setInterval, clearInterval } = intervalProvider; + const { setTimeout } = timeoutProvider; const handle = setInterval(() => { values.push(`a@${testScheduler.now()}`); clearInterval(handle); }, 0); - setImmediate(() => { + setTimeout(() => { values.push(`b@${testScheduler.now()}`); + }, 0); + setImmediate(() => { + values.push(`c@${testScheduler.now()}`); }); expect(values).to.deep.equal([]); testScheduler.schedule(() => { - expect(values).to.deep.equal(['b@0', 'a@0']); + expect(values).to.deep.equal(['c@0', 'a@0', 'b@0']); }, 10); }); }); diff --git a/spec/util/canReportError-spec.ts b/spec/util/canReportError-spec.ts deleted file mode 100644 index dec8869e61..0000000000 --- a/spec/util/canReportError-spec.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { expect } from 'chai'; -import { noop, Subscriber } from 'rxjs'; -import { canReportError } from 'rxjs/internal/Observable'; -import { SafeSubscriber } from 'rxjs/internal/Subscriber'; - -describe('canReportError', () => { - it('should report errors to an observer if possible', () => { - const subscriber = new SafeSubscriber(noop, noop); - expect(canReportError(subscriber)).to.be.true; - }); - - it('should not report errors to a stopped observer', () => { - const subscriber = new SafeSubscriber(noop, noop); - subscriber.error(new Error('kaboom')); - expect(canReportError(subscriber)).to.be.false; - }); - - it('should not report errors an observer with a stopped destination', () => { - const destination = new SafeSubscriber(noop, noop); - const subscriber = new Subscriber(destination); - destination.error(new Error('kaboom')); - expect(canReportError(subscriber)).to.be.false; - }); -}); diff --git a/src/internal/Notification.ts b/src/internal/Notification.ts index 0062de6e51..8302a1359d 100644 --- a/src/internal/Notification.ts +++ b/src/internal/Notification.ts @@ -238,42 +238,3 @@ export function observeNotification(notification: ObservableNotification, } kind === 'N' ? observer.next?.(value!) : kind === 'E' ? observer.error?.(error) : observer.complete?.(); } - -/** - * A completion object optimized for memory use and created to be the - * same "shape" as other notifications in v8. - * @internal - */ -export const COMPLETE_NOTIFICATION = (() => createNotification('C', undefined, undefined) as CompleteNotification)(); - -/** - * Internal use only. Creates an optimized error notification that is the same "shape" - * as other notifications. - * @internal - */ -export function errorNotification(error: any): ErrorNotification { - return createNotification('E', undefined, error) as any; -} - -/** - * Internal use only. Creates an optimized next notification that is the same "shape" - * as other notifications. - * @internal - */ -export function nextNotification(value: T) { - return createNotification('N', value, undefined) as NextNotification; -} - -/** - * Ensures that all notifications created internally have the same "shape" in v8. - * - * TODO: This is only exported to support a crazy legacy test in `groupBy`. - * @internal - */ -export function createNotification(kind: 'N' | 'E' | 'C', value: any, error: any) { - return { - kind, - value, - error, - }; -} diff --git a/src/internal/NotificationFactories.ts b/src/internal/NotificationFactories.ts new file mode 100644 index 0000000000..bdf79c1b97 --- /dev/null +++ b/src/internal/NotificationFactories.ts @@ -0,0 +1,41 @@ +/** @prettier */ +import { CompleteNotification, NextNotification, ErrorNotification } from './types'; + +/** + * A completion object optimized for memory use and created to be the + * same "shape" as other notifications in v8. + * @internal + */ +export const COMPLETE_NOTIFICATION = (() => createNotification('C', undefined, undefined) as CompleteNotification)(); + +/** + * Internal use only. Creates an optimized error notification that is the same "shape" + * as other notifications. + * @internal + */ +export function errorNotification(error: any): ErrorNotification { + return createNotification('E', undefined, error) as any; +} + +/** + * Internal use only. Creates an optimized next notification that is the same "shape" + * as other notifications. + * @internal + */ +export function nextNotification(value: T) { + return createNotification('N', value, undefined) as NextNotification; +} + +/** + * Ensures that all notifications created internally have the same "shape" in v8. + * + * TODO: This is only exported to support a crazy legacy test in `groupBy`. + * @internal + */ +export function createNotification(kind: 'N' | 'E' | 'C', value: any, error: any) { + return { + kind, + value, + error, + }; +} diff --git a/src/internal/Observable.ts b/src/internal/Observable.ts index bdb3d5311a..ea64715de8 100644 --- a/src/internal/Observable.ts +++ b/src/internal/Observable.ts @@ -8,7 +8,6 @@ import { TeardownLogic, OperatorFunction, PartialObserver, Subscribable, Observe import { observable as Symbol_observable } from './symbol/observable'; import { pipeFromArray } from './util/pipe'; import { config } from './config'; -import { reportUnhandledError } from './util/reportUnhandledError'; import { isFunction } from './util/isFunction'; /** @@ -235,11 +234,8 @@ export class Observable implements Subscribable { } catch (err) { if (config.useDeprecatedSynchronousErrorHandling) { throw err; - } else { - // If an error is thrown during subscribe, but our subscriber is closed, so we cannot notify via the - // subscription "error" channel, it is an unhandled error and we need to report it appropriately. - canReportError(sink) ? sink.error(err) : reportUnhandledError(err); } + sink.error(err); } } @@ -484,23 +480,6 @@ function getPromiseCtor(promiseCtor: PromiseConstructorLike | undefined) { return promiseCtor ?? config.Promise ?? Promise; } -/** - * Determines whether the subscriber is closed or stopped or has a - * destination that is closed or stopped - in which case errors will - * need to be reported via a different mechanism. - * @param subscriber the subscriber to check - */ -export function canReportError(subscriber: Subscriber): boolean { - while (subscriber) { - const { closed, destination, isStopped } = subscriber as any; - if (closed || isStopped) { - return false; - } - subscriber = destination && destination instanceof Subscriber ? destination : null!; - } - return true; -} - function isObserver(value: any): value is Observer { return value && isFunction(value.next) && isFunction(value.error) && isFunction(value.complete); } diff --git a/src/internal/Subscriber.ts b/src/internal/Subscriber.ts index 478bb4146a..637b1e58ce 100644 --- a/src/internal/Subscriber.ts +++ b/src/internal/Subscriber.ts @@ -1,10 +1,12 @@ /** @prettier */ import { isFunction } from './util/isFunction'; -import { Observer, PartialObserver } from './types'; +import { Observer, PartialObserver, ObservableNotification } from './types'; import { isSubscription, Subscription } from './Subscription'; import { config } from './config'; import { reportUnhandledError } from './util/reportUnhandledError'; import { noop } from './util/noop'; +import { nextNotification, errorNotification, COMPLETE_NOTIFICATION } from './NotificationFactories'; +import { timeoutProvider } from './scheduler/timeoutProvider'; /** * Implements the {@link Observer} interface and extends the @@ -62,7 +64,9 @@ export class Subscriber extends Subscription implements Observer { * @return {void} */ next(value?: T): void { - if (!this.isStopped) { + if (this.isStopped) { + handleStoppedNotification(nextNotification(value), this); + } else { this._next(value!); } } @@ -75,7 +79,9 @@ export class Subscriber extends Subscription implements Observer { * @return {void} */ error(err?: any): void { - if (!this.isStopped) { + if (this.isStopped) { + handleStoppedNotification(errorNotification(err), this); + } else { this.isStopped = true; this._error(err); } @@ -88,7 +94,9 @@ export class Subscriber extends Subscription implements Observer { * @return {void} */ complete(): void { - if (!this.isStopped) { + if (this.isStopped) { + handleStoppedNotification(COMPLETE_NOTIFICATION, this); + } else { this.isStopped = true; this._complete(); } @@ -181,6 +189,16 @@ function defaultErrorHandler(err: any) { reportUnhandledError(err); } +/** + * A handler for notifications that cannot be sent to a stopped subscriber. + * @param notification The notification being sent + * @param subscriber The stopped subscriber + */ +function handleStoppedNotification(notification: ObservableNotification, subscriber: Subscriber) { + const { onStoppedNotification } = config; + onStoppedNotification && timeoutProvider.setTimeout(() => onStoppedNotification(notification, subscriber)); +} + /** * The observer used as a stub for subscriptions where the user did not * pass any arguments to `subscribe`. Comes with the default error handling diff --git a/src/internal/config.ts b/src/internal/config.ts index a87f35fd4e..9f4bacd2dc 100644 --- a/src/internal/config.ts +++ b/src/internal/config.ts @@ -1,8 +1,10 @@ /** @prettier */ +import { Subscriber } from './Subscriber'; +import { ObservableNotification } from './types'; /** * The global configuration object for RxJS, used to configure things - * like what Promise contructor should used to create Promises + * like what Promise constructor should used to create Promises */ export const config = { /** @@ -16,6 +18,18 @@ export const config = { */ onUnhandledError: null as ((err: any) => void) | null, + /** + * A registration point for notifications that cannot be sent to subscribers because they + * have completed, errored or have been explicitly unsubscribed. By default, next, complete + * and error notifications sent to stopped subscribers are noops. However, sometimes callers + * might want a different behavior. For example, with sources that attempt to report errors + * to stopped subscribers, a caller can configure RxJS to throw an unhandled error instead. + * This will _always_ be called asynchronously on another job in the runtime. This is because + * we do not want errors thrown in this user-configured handler to interfere with the + * behavior of the library. + */ + onStoppedNotification: null as ((notification: ObservableNotification, subscriber: Subscriber) => void) | null, + /** * The promise constructor used by default for methods such as * {@link toPromise} and {@link forEach} @@ -24,7 +38,7 @@ export const config = { * Promise constructor. If you need a Promise implementation other than native promises, * please polyfill/patch Promises as you see appropriate. */ - Promise: undefined! as PromiseConstructorLike, + Promise: undefined as PromiseConstructorLike | undefined, /** * If true, turns on synchronous error rethrowing, which is a deprecated behavior diff --git a/src/internal/scheduler/timeoutProvider.ts b/src/internal/scheduler/timeoutProvider.ts new file mode 100644 index 0000000000..aa76be0b28 --- /dev/null +++ b/src/internal/scheduler/timeoutProvider.ts @@ -0,0 +1,28 @@ +/** @prettier */ +type SetTimeoutFunction = (handler: () => void, timeout?: number, ...args: any[]) => number; +type ClearTimeoutFunction = (handle: number) => void; + +interface TimeoutProvider { + setTimeout: SetTimeoutFunction; + clearTimeout: ClearTimeoutFunction; + delegate: + | { + setTimeout: SetTimeoutFunction; + clearTimeout: ClearTimeoutFunction; + } + | undefined; +} + +export const timeoutProvider: TimeoutProvider = { + // When accessing the delegate, use the variable rather than `this` so that + // the functions can be called without being bound to the provider. + setTimeout(...args) { + const { delegate } = timeoutProvider; + return (delegate?.setTimeout || setTimeout)(...args); + }, + clearTimeout(handle) { + const { delegate } = timeoutProvider; + return (delegate?.clearTimeout || clearTimeout)(handle); + }, + delegate: undefined, +}; diff --git a/src/internal/testing/TestScheduler.ts b/src/internal/testing/TestScheduler.ts index 41468184db..254d36f051 100644 --- a/src/internal/testing/TestScheduler.ts +++ b/src/internal/testing/TestScheduler.ts @@ -6,12 +6,13 @@ import { SubscriptionLog } from './SubscriptionLog'; import { Subscription } from '../Subscription'; import { VirtualTimeScheduler, VirtualAction } from '../scheduler/VirtualTimeScheduler'; import { ObservableNotification } from '../types'; -import { COMPLETE_NOTIFICATION, errorNotification, nextNotification } from '../Notification'; +import { COMPLETE_NOTIFICATION, errorNotification, nextNotification } from '../NotificationFactories'; import { dateTimestampProvider } from '../scheduler/dateTimestampProvider'; import { performanceTimestampProvider } from '../scheduler/performanceTimestampProvider'; import { animationFrameProvider } from '../scheduler/animationFrameProvider'; import { immediateProvider } from '../scheduler/immediateProvider'; import { intervalProvider } from '../scheduler/intervalProvider'; +import { timeoutProvider } from '../scheduler/timeoutProvider'; const defaultMaxFrame: number = 750; @@ -488,19 +489,19 @@ export class TestScheduler extends VirtualTimeScheduler { handle: number; handler: () => void; subscription: Subscription; - type: 'immediate' | 'interval'; + type: 'immediate' | 'interval' | 'timeout'; }>(); const run = () => { // Whenever a scheduled run is executed, it must run a single immediate // or interval action - with immediate actions being prioritized over - // interval actions. + // interval and timeout actions. const now = this.now(); const scheduledRecords = Array.from(scheduleLookup.values()); const scheduledRecordsDue = scheduledRecords.filter(({ due }) => due <= now); - const immediates = scheduledRecordsDue.filter(({ type }) => type === 'immediate'); - if (immediates.length > 0) { - const { handle, handler } = immediates[0]; + const dueImmediates = scheduledRecordsDue.filter(({ type }) => type === 'immediate'); + if (dueImmediates.length > 0) { + const { handle, handler } = dueImmediates[0]; scheduleLookup.delete(handle); handler(); return; @@ -517,9 +518,28 @@ export class TestScheduler extends VirtualTimeScheduler { handler(); return; } + const dueTimeouts = scheduledRecordsDue.filter(({ type }) => type === 'timeout'); + if (dueTimeouts.length > 0) { + const { handle, handler } = dueTimeouts[0]; + scheduleLookup.delete(handle); + handler(); + return; + } throw new Error('Expected a due immediate or interval'); }; + // The following objects are the delegates that replace conventional + // runtime implementations with TestScheduler implementations. + // + // The immediate delegate is depended upon by the asapScheduler. + // + // The interval delegate is depended upon by the asyncScheduler. + // + // The timeout delegate is not depended upon by any scheduler, but it's + // included here because the onUnhandledError and onStoppedNotification + // configuration points use setTimeout to avoid producer interference. It's + // inclusion allows for the testing of these configuration points. + const immediate = { setImmediate: (handler: () => void) => { const handle = ++lastHandle; @@ -564,7 +584,29 @@ export class TestScheduler extends VirtualTimeScheduler { } }; - return { immediate, interval }; + const timeout = { + setTimeout: (handler: () => void, duration = 0) => { + const handle = ++lastHandle; + scheduleLookup.set(handle, { + due: this.now() + duration, + duration, + handle, + handler, + subscription: this.schedule(run, duration), + type: 'timeout', + }); + return handle; + }, + clearTimeout: (handle: number) => { + const value = scheduleLookup.get(handle); + if (value) { + value.subscription.unsubscribe(); + scheduleLookup.delete(handle); + } + } + }; + + return { immediate, interval, timeout }; } /** @@ -590,6 +632,7 @@ export class TestScheduler extends VirtualTimeScheduler { dateTimestampProvider.delegate = this; immediateProvider.delegate = delegates.immediate; intervalProvider.delegate = delegates.interval; + timeoutProvider.delegate = delegates.timeout; performanceTimestampProvider.delegate = this; const helpers: RunHelpers = { @@ -613,6 +656,7 @@ export class TestScheduler extends VirtualTimeScheduler { dateTimestampProvider.delegate = undefined; immediateProvider.delegate = undefined; intervalProvider.delegate = undefined; + timeoutProvider.delegate = undefined; performanceTimestampProvider.delegate = undefined; } } diff --git a/src/internal/util/reportUnhandledError.ts b/src/internal/util/reportUnhandledError.ts index ab670538a0..cf7e4377c6 100644 --- a/src/internal/util/reportUnhandledError.ts +++ b/src/internal/util/reportUnhandledError.ts @@ -1,5 +1,6 @@ /** @prettier */ import { config } from '../config'; +import { timeoutProvider } from '../scheduler/timeoutProvider'; /** * Handles an error on another job either with the user-configured {@link onUnhandledError}, @@ -11,7 +12,7 @@ import { config } from '../config'; * @param err the error to report */ export function reportUnhandledError(err: any) { - setTimeout(() => { + timeoutProvider.setTimeout(() => { const { onUnhandledError } = config; if (onUnhandledError) { // Execute the user-configured error handler.