Skip to content
This repository was archived by the owner on Feb 26, 2024. It is now read-only.

fix: simulated HTTP errors now delayed as expected #182

Merged
merged 1 commit into from
Mar 9, 2018
Merged

Conversation

wardbell
Copy link
Contributor

@wardbell wardbell commented Mar 9, 2018

Fixes bug in which a simulated HTTP error did NOT honor the prescribed latency delay.
New makeResponseDelay() function cures that.
Added tests for this.

Library no longer uses RxJS delay() which may make userland tests easier because Angular TestBed "hates" RxJS delay() (it uses interval()).

Also closes #180.

When this PR is merged, can publish the new version to NPM as release 0.5.4.

@wardbell wardbell self-assigned this Mar 9, 2018
@wardbell wardbell requested a review from IgorMinar March 9, 2018 20:30
Copy link
Contributor

@IgorMinar IgorMinar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add docs - otherwise lgtm

@@ -0,0 +1,30 @@
import { Observable } from 'rxjs/Observable';

/** adds specified delay (in ms) to both next and error channels of the response observable */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add docs you wrote for changelog also here so that nobody reading the code is wondering why we have a custom delay operator.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added reference to the CHANGELOG

CHANGELOG.md Outdated
<a id="0.5.4"></a>
## 0.5.4 (2018-03-09)

Simulated HTTP error responses were not delaying the prescribed time when using RxJS `delay()`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Describes the change to the implementation of simulated delay

@@ -160,7 +160,7 @@ export abstract class BackendService {
*/
protected addDelay(response: Observable<any>): Observable<any> {
const d = this.config.delay;
return d === 0 ? response : delay.call(response, d || 500);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaces RxJS delay() (incorrect for errors) with new delayRespons() function.

@wardbell
Copy link
Contributor Author

wardbell commented Mar 9, 2018

Piggy backed fix to issue #180

@wardbell wardbell merged commit 25c6a21 into master Mar 9, 2018
@wardbell wardbell deleted the fix-delay branch March 9, 2018 22:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Class 'HttpClientBackendService' incorrectly implements interface 'HttpBackend'.
2 participants