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

fix(timeout): throw traceable TimeoutError #2132

Merged
merged 1 commit into from
Nov 17, 2016

Conversation

kwonoj
Copy link
Member

@kwonoj kwonoj commented Nov 15, 2016

Description:
This PR updates behaivor of timeout operator for better readibility of stack trace when TimeoutError thrown.

For example, with given code snippet

Rx.Observable.never().timeout(10).subscribe(console.log.bind(console));

stacktrace will be like below

D:\github\rxjs\dist\cjs\scheduler\AsyncScheduler.js:45
            throw error;
            ^
TimeoutError: Timeout has occurred
    at new TimeoutError (D:\github\rxjs\dist\cjs\util\TimeoutError.js:17:26)
    at TimeoutSubscriber.notifyTimeout (D:\github\rxjs\dist\cjs\operator\timeout.js:99:40)
    at AsyncAction.TimeoutSubscriber.dispatchTimeout [as work] (D:\github\rxjs\dist\cjs\operator\timeout.js:75:20)
    at AsyncAction._execute (D:\github\rxjs\dist\cjs\scheduler\AsyncAction.js:111:18)
    at AsyncAction.execute (D:\github\rxjs\dist\cjs\scheduler\AsyncAction.js:86:26)
    at AsyncScheduler.flush (D:\github\rxjs\dist\cjs\scheduler\AsyncScheduler.js:36:32)
    at ontimeout (timers.js:365:14)
    at tryOnTimeout (timers.js:237:5)
    at Timer.listOnTimeout (timers.js:207:5)

which loses original context of code but shows stack trace of async execution context (vary by scheduler specified). This makes hard to track down unhandled exception since it's disconnected to original codebases. Instead, this PR creates error to contain stack traces nearby originall caller, emits like below

D:\github\rxjs\dist\cjs\scheduler\AsyncScheduler.js:45
            throw error;
            ^
TimeoutError: Timeout has occurred
    at new TimeoutError (D:\github\rxjs\dist\cjs\util\TimeoutError.js:17:26)
    at NeverObservable.timeout (D:\github\rxjs\dist\cjs\operator\timeout.js:24:32)
    at Object.<anonymous> (D:\github\rxjs\dummy.js:3:23)
    at Module._compile (module.js:573:32)
    at Object.Module._extensions..js (module.js:582:10)
    at Module.load (module.js:490:32)
    at tryModuleLoad (module.js:449:12)
    at Function.Module._load (module.js:441:3)
    at Module.runMain (module.js:607:10)
    at run (bootstrap_node.js:382:7)

allows bit more easier tracking.

Related issue (if exists):

- create TimeoutError contain stacktrace nearby caller instead of scheduler
@kwonoj kwonoj added the feature PRs and issues for features label Nov 15, 2016
@coveralls
Copy link

coveralls commented Nov 15, 2016

Coverage Status

Coverage increased (+0.0004%) to 97.686% when pulling 29bbd60 on kwonoj:fix-timeout into e0cf882 on ReactiveX:master.

@jayphelps
Copy link
Member

wow! this is awesome! LGTM

@lock
Copy link

lock bot commented Jun 6, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature PRs and issues for features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants