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

Do not cleanup subscription if next throws #123

Merged
merged 2 commits into from
Dec 5, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion spec/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ <h1>Subscription Observer Objects</h1>
<ul>
<li>If the observer's `error` method is called, the observer will not be invoked again and the observable's cleanup function will be called.</li>
<li>If the observer's `complete` method is called, the observer will not be invoked again and the observable's cleanup function will be called.</li>
<li>If the observer throws an exception, the observable's cleanup function will be called.</li>
<li>When the subscription is canceled, the observer will not be invoked again.</li>
</ul>

Expand Down
12 changes: 3 additions & 9 deletions spec/subscription-observer.html
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,9 @@ <h1>%SubscriptionObserverPrototype%.next ( _value_ )</h1>
1. If SubscriptionClosed(_subscription_) is *true*, return *undefined*.
1. Let _observer_ be the value of _subscription_'s [[Observer]] internal slot.
1. Assert: Type(_observer_) is Object.
1. Let _result_ be GetMethod(_observer_, `"next"`).
1. If _result_ is not an abrupt completion, then
1. Let _nextMethod_ be _result_.[[value]].
1. If _nextMethod_ is *undefined*, let _result_ be NormalCompletion(*undefined*).
1. Else, let _result_ be Call(_nextMethod_, _observer_, « ‍_value_ »).
1. If _result_ is an abrupt completion, then
1. Set _subscription_'s [[Observer]] internal slot to *undefined*.
1. Perform ? CleanupSubscription(_subscription_).
1. Return Completion(_result_).
1. Let _nextMethod_ be ? GetMethod(_observer_, `"next"`).
1. If _nextMethod_ is *undefined*, let _result_ be NormalCompletion(*undefined*).
1. Return ? Call(_nextMethod_, _observer_, « ‍_value_ »).
</emu-alg>
</emu-clause>

Expand Down
22 changes: 6 additions & 16 deletions src/Observable.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,24 +145,14 @@ SubscriptionObserver.prototype = nonEnum({
return undefined;

let observer = subscription._observer;
let m = getMethod(observer, "next");

try {

let m = getMethod(observer, "next");

// If the observer doesn't support "next", then return undefined
if (!m)
return undefined;

// Send the next value to the sink
return m.call(observer, value);

} catch (e) {
// If the observer doesn't support "next", then return undefined
if (!m)
return undefined;

// If the observer throws, then close the stream and rethrow the error
try { closeSubscription(subscription) }
finally { throw e }
}
// Send the next value to the sink
return m.call(observer, value);
},

error(value) {
Expand Down
21 changes: 5 additions & 16 deletions test/observer-next.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,25 +113,14 @@ export default {
});

called = 0;
observable.subscribe({ next() { throw new Error() } });
let subscription = observable.subscribe({ next() { throw new Error() } });
try { observer.next() }
catch (x) {}
test._("Cleanup function is called when next throws an error")
.equals(called, 1);

let error = new Error(), caught = null;

new Observable(x => {
observer = x;
return _=> { throw new Error() };
}).subscribe({ next() { throw error } });

try { observer.next() }
catch (x) { caught = x }
test._("Cleanup function is not called when next throws an error")
.equals(called, 0);

test._("If both next and the cleanup function throw, then the error " +
"from the next method is thrown")
.assert(caught === error);
test._("Subscription is not closed when next throws an error")
.equals(subscription.closed, false);

},

Expand Down