feat(timeout): remove `errorToSend` argument, always throw TimeoutError #2172

benlesh · 2016-12-05T23:23:42Z

The errorToSend argument did not make much sense, so we are removing it ahead of the 5.0 release. All timeout errors will be thrown as TimeoutError, which can be tested with an assertion against instanceof Rx.TimeoutError.

BREAKING CHANGE: timeout no longer accepts the errorToSend argument

related #2141

benlesh · 2016-12-05T23:24:06Z

cc @kwonoj @jayphelps

coveralls · 2016-12-05T23:28:30Z

Coverage decreased (-0.001%) to 97.685% when pulling 5854691 on blesh:timeout_fix into bd56b3c on ReactiveX:master.

kwonoj · 2016-12-05T23:29:03Z

src/operator/timeout.ts

@@ -97,6 +93,6 @@ class TimeoutSubscriber<T> extends Subscriber<T> {
  }

  notifyTimeout(): void {
-    this.error(this.errorToSend);
+    this.error(new TimeoutError());


@Blesh I'd propose to create error object in ctor, allows bit easier stack to trace (was done in #2132)

Why allocate an object you don't need?

Not sure which object is unused, pr #2132 is simply changing where error object is created and pass into here. Would you elaborate bit what I'm missing?

@Blesh because if you do end up needing it, the stack trace is awful if we don't do it at the constructor call site. See #2132

I see. Makes sense.

benlesh · 2016-12-07T00:47:52Z

The coverage results seem to be just the unfortunate math of removing a few tests we didn't need. I'm not worried about it.

The `errorToSend` argument did not make much sense, so we are removing it ahead of the 5.0 release. All timeout errors will be thrown as `TimeoutError`, which can be tested with an assertion against `instanceof Rx.TimeoutError`. BREAKING CHANGE: `timeout` no longer accepts the `errorToSend` argument related #2141

kwonoj · 2016-12-07T01:14:38Z

LGTM

coveralls · 2016-12-07T04:24:11Z

Coverage decreased (-0.001%) to 97.685% when pulling 5854691 on blesh:timeout_fix into bd56b3c on ReactiveX:master.

samal-rasmussen · 2017-06-02T12:29:04Z

Note to anyone else who was using the errorToSend argument to create a contextual error message.
You can use catch immediately after the timeout and check the error for instanceof TimeoutError like this to achive the same:

source.timeout(1000)
.catch((error) => {
  if (error instanceof TimeoutError) {
    throw new Error('Timeout error. Value of someLocalVariable was: ' + someLocalVariable);
  } else {
    throw error;
  }
});

Quite a lot more lines I know.

samal-rasmussen · 2017-06-02T12:34:29Z

Also @benlesh why did the errorToSend argument not make any sense?

lock · 2018-06-06T09:41:17Z

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.

benlesh mentioned this pull request Dec 5, 2016

fix(timeout): wrap customError with TimeoutError #2141

Closed

kwonoj reviewed Dec 5, 2016

View reviewed changes

benlesh merged commit 98ea3d2 into ReactiveX:master Dec 7, 2016

benlesh deleted the timeout_fix branch December 16, 2016 03:15

lock bot locked as resolved and limited conversation to collaborators Jun 6, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(timeout): remove `errorToSend` argument, always throw TimeoutError #2172

feat(timeout): remove `errorToSend` argument, always throw TimeoutError #2172

benlesh commented Dec 5, 2016

benlesh commented Dec 5, 2016

coveralls commented Dec 5, 2016 •

edited

Loading

kwonoj Dec 5, 2016 •

edited by jayphelps

Loading

benlesh Dec 5, 2016

kwonoj Dec 5, 2016

jayphelps Dec 5, 2016

benlesh Dec 7, 2016

benlesh commented Dec 7, 2016

kwonoj commented Dec 7, 2016

coveralls commented Dec 7, 2016 •

edited

Loading

samal-rasmussen commented Jun 2, 2017

samal-rasmussen commented Jun 2, 2017

lock bot commented Jun 6, 2018

feat(timeout): remove errorToSend argument, always throw TimeoutError #2172

feat(timeout): remove errorToSend argument, always throw TimeoutError #2172

Conversation

benlesh commented Dec 5, 2016

benlesh commented Dec 5, 2016

coveralls commented Dec 5, 2016 • edited Loading

kwonoj Dec 5, 2016 • edited by jayphelps Loading

Choose a reason for hiding this comment

benlesh Dec 5, 2016

Choose a reason for hiding this comment

kwonoj Dec 5, 2016

Choose a reason for hiding this comment

jayphelps Dec 5, 2016

Choose a reason for hiding this comment

benlesh Dec 7, 2016

Choose a reason for hiding this comment

benlesh commented Dec 7, 2016

kwonoj commented Dec 7, 2016

coveralls commented Dec 7, 2016 • edited Loading

samal-rasmussen commented Jun 2, 2017

samal-rasmussen commented Jun 2, 2017

lock bot commented Jun 6, 2018

feat(timeout): remove `errorToSend` argument, always throw TimeoutError #2172

feat(timeout): remove `errorToSend` argument, always throw TimeoutError #2172

coveralls commented Dec 5, 2016 •

edited

Loading

kwonoj Dec 5, 2016 •

edited by jayphelps

Loading

coveralls commented Dec 7, 2016 •

edited

Loading