From 23d77d448cf5f8637e6b575b08f6f735cb665491 Mon Sep 17 00:00:00 2001 From: Rob Wormald Date: Thu, 29 Oct 2015 17:50:12 -0700 Subject: [PATCH 1/2] fix(http): use Response for JSONP errors Return Response when JSONP backend errors --- .../src/http/backends/jsonp_backend.ts | 21 +++++++++++++++---- .../test/http/backends/jsonp_backend_spec.ts | 4 ++-- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/modules/angular2/src/http/backends/jsonp_backend.ts b/modules/angular2/src/http/backends/jsonp_backend.ts index fb1390bce082..80a76ea9a09b 100644 --- a/modules/angular2/src/http/backends/jsonp_backend.ts +++ b/modules/angular2/src/http/backends/jsonp_backend.ts @@ -1,5 +1,5 @@ import {ConnectionBackend, Connection} from '../interfaces'; -import {ReadyStates, RequestMethods} from '../enums'; +import {ReadyStates, RequestMethods, ResponseTypes} from '../enums'; import {Request} from '../static_request'; import {Response} from '../static_response'; import {ResponseOptions, BaseResponseOptions} from '../base_response_options'; @@ -11,6 +11,10 @@ import {StringWrapper, isPresent} from 'angular2/src/core/facade/lang'; // todo(robwormald): temporary until https://github.com/angular/angular/issues/4390 decided var Rx = require('@reactivex/rxjs/dist/cjs/Rx'); var {Observable} = Rx; + +const JSONP_ERR_NO_CALLBACK = 'JSONP injected script did not invoke callback.'; +const JSONP_ERR_WRONG_METHOD = 'JSONP requests must use GET request method.'; + export abstract class JSONPConnection implements Connection { readyState: ReadyStates; request: Request; @@ -28,7 +32,7 @@ export class JSONPConnection_ extends JSONPConnection { private baseResponseOptions?: ResponseOptions) { super(); if (req.method !== RequestMethods.Get) { - throw makeTypeError("JSONP requests must use GET request method."); + throw makeTypeError(JSONP_ERR_WRONG_METHOD); } this.request = req; this.response = new Observable(responseObserver => { @@ -56,7 +60,12 @@ export class JSONPConnection_ extends JSONPConnection { this.readyState = ReadyStates.Done; _dom.cleanup(script); if (!this._finished) { - responseObserver.error(makeTypeError('JSONP injected script did not invoke callback.')); + let responseOptions = + new ResponseOptions({body: JSONP_ERR_NO_CALLBACK, type: ResponseTypes.Error}); + if (isPresent(baseResponseOptions)) { + responseOptions = baseResponseOptions.merge(responseOptions); + } + responseObserver.error(new Response(responseOptions)); return; } @@ -73,7 +82,11 @@ export class JSONPConnection_ extends JSONPConnection { if (this.readyState === ReadyStates.Cancelled) return; this.readyState = ReadyStates.Done; _dom.cleanup(script); - responseObserver.error(error); + let responseOptions = new ResponseOptions({body: error.message, type: ResponseTypes.Error}); + if (isPresent(baseResponseOptions)) { + responseOptions = baseResponseOptions.merge(responseOptions); + } + responseObserver.error(new Response(responseOptions)); }; script.addEventListener('load', onLoad); diff --git a/modules/angular2/test/http/backends/jsonp_backend_spec.ts b/modules/angular2/test/http/backends/jsonp_backend_spec.ts index b3bc437de307..9476d8c40378 100644 --- a/modules/angular2/test/http/backends/jsonp_backend_spec.ts +++ b/modules/angular2/test/http/backends/jsonp_backend_spec.ts @@ -134,7 +134,7 @@ export function main() { async.done(); }, err => { - expect(StringWrapper.contains(err.message, 'did not invoke callback')).toBe(true); + expect(err.text()).toEqual('JSONP injected script did not invoke callback.'); async.done(); }); @@ -150,7 +150,7 @@ export function main() { async.done(); }, err => { - expect(err['message']).toBe('Oops!'); + expect(err.text()).toBe('Oops!'); async.done(); }); From db40f9af30f4e2a2d71b3a47edfa7d010c9f7702 Mon Sep 17 00:00:00 2001 From: Rob Wormald Date: Fri, 30 Oct 2015 23:52:37 -0700 Subject: [PATCH 2/2] fix(http): use Observable on Http methods use correct type definitions for Http responses. --- .../src/http/backends/jsonp_backend.ts | 7 ++----- .../angular2/src/http/backends/xhr_backend.ts | 6 ++---- modules/angular2/src/http/http.ts | 20 ++++++++++--------- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/modules/angular2/src/http/backends/jsonp_backend.ts b/modules/angular2/src/http/backends/jsonp_backend.ts index 80a76ea9a09b..d3a56ea1ea75 100644 --- a/modules/angular2/src/http/backends/jsonp_backend.ts +++ b/modules/angular2/src/http/backends/jsonp_backend.ts @@ -5,12 +5,9 @@ import {Response} from '../static_response'; import {ResponseOptions, BaseResponseOptions} from '../base_response_options'; import {Injectable} from 'angular2/angular2'; import {BrowserJsonp} from './browser_jsonp'; -import {EventEmitter, ObservableWrapper} from 'angular2/src/core/facade/async'; import {makeTypeError} from 'angular2/src/core/facade/exceptions'; import {StringWrapper, isPresent} from 'angular2/src/core/facade/lang'; -// todo(robwormald): temporary until https://github.com/angular/angular/issues/4390 decided -var Rx = require('@reactivex/rxjs/dist/cjs/Rx'); -var {Observable} = Rx; +import {Observable} from 'angular2/angular2'; const JSONP_ERR_NO_CALLBACK = 'JSONP injected script did not invoke callback.'; const JSONP_ERR_WRONG_METHOD = 'JSONP requests must use GET request method.'; @@ -18,7 +15,7 @@ const JSONP_ERR_WRONG_METHOD = 'JSONP requests must use GET request method.'; export abstract class JSONPConnection implements Connection { readyState: ReadyStates; request: Request; - response: any; + response: Observable; abstract finished(data?: any): void; } diff --git a/modules/angular2/src/http/backends/xhr_backend.ts b/modules/angular2/src/http/backends/xhr_backend.ts index 371b9965a0c8..19f5cb8c5a53 100644 --- a/modules/angular2/src/http/backends/xhr_backend.ts +++ b/modules/angular2/src/http/backends/xhr_backend.ts @@ -6,9 +6,7 @@ import {ResponseOptions, BaseResponseOptions} from '../base_response_options'; import {Injectable} from 'angular2/angular2'; import {BrowserXhr} from './browser_xhr'; import {isPresent} from 'angular2/src/core/facade/lang'; -// todo(robwormald): temporary until https://github.com/angular/angular/issues/4390 decided -var Rx = require('@reactivex/rxjs/dist/cjs/Rx'); -var {Observable} = Rx; +import {Observable} from 'angular2/angular2'; /** * Creates connections using `XMLHttpRequest`. Given a fully-qualified * request, an `XHRConnection` will immediately create an `XMLHttpRequest` object and send the @@ -23,7 +21,7 @@ export class XHRConnection implements Connection { * Response {@link EventEmitter} which emits a single {@link Response} value on load event of * `XMLHttpRequest`. */ - response: any; // TODO: Make generic of ; + response: Observable; readyState: ReadyStates; constructor(req: Request, browserXHR: BrowserXhr, baseResponseOptions?: ResponseOptions) { this.request = req; diff --git a/modules/angular2/src/http/http.ts b/modules/angular2/src/http/http.ts index cd7ea0214111..bccbf3f66062 100644 --- a/modules/angular2/src/http/http.ts +++ b/modules/angular2/src/http/http.ts @@ -3,10 +3,12 @@ import {makeTypeError} from 'angular2/src/core/facade/exceptions'; import {Injectable} from 'angular2/angular2'; import {RequestOptionsArgs, Connection, ConnectionBackend} from './interfaces'; import {Request} from './static_request'; +import {Response} from './static_response'; import {BaseRequestOptions, RequestOptions} from './base_request_options'; import {RequestMethods} from './enums'; +import {Observable} from 'angular2/angular2'; -function httpRequest(backend: ConnectionBackend, request: Request): any { +function httpRequest(backend: ConnectionBackend, request: Request): Observable { return backend.createConnection(request).response; } @@ -96,7 +98,7 @@ export class Http { * object can be provided as the 2nd argument. The options object will be merged with the values * of {@link BaseRequestOptions} before performing the request. */ - request(url: string | Request, options?: RequestOptionsArgs): any { + request(url: string | Request, options?: RequestOptionsArgs): Observable { var responseObservable: any; if (isString(url)) { responseObservable = httpRequest( @@ -113,7 +115,7 @@ export class Http { /** * Performs a request with `get` http method. */ - get(url: string, options?: RequestOptionsArgs): any { + get(url: string, options?: RequestOptionsArgs): Observable { return httpRequest(this._backend, new Request(mergeOptions(this._defaultOptions, options, RequestMethods.Get, url))); } @@ -121,7 +123,7 @@ export class Http { /** * Performs a request with `post` http method. */ - post(url: string, body: string, options?: RequestOptionsArgs): any { + post(url: string, body: string, options?: RequestOptionsArgs): Observable { return httpRequest( this._backend, new Request(mergeOptions(this._defaultOptions.merge(new RequestOptions({body: body})), @@ -131,7 +133,7 @@ export class Http { /** * Performs a request with `put` http method. */ - put(url: string, body: string, options?: RequestOptionsArgs): any { + put(url: string, body: string, options?: RequestOptionsArgs): Observable { return httpRequest( this._backend, new Request(mergeOptions(this._defaultOptions.merge(new RequestOptions({body: body})), @@ -141,7 +143,7 @@ export class Http { /** * Performs a request with `delete` http method. */ - delete (url: string, options?: RequestOptionsArgs): any { + delete (url: string, options?: RequestOptionsArgs): Observable { return httpRequest(this._backend, new Request(mergeOptions(this._defaultOptions, options, RequestMethods.Delete, url))); } @@ -149,7 +151,7 @@ export class Http { /** * Performs a request with `patch` http method. */ - patch(url: string, body: string, options?: RequestOptionsArgs): any { + patch(url: string, body: string, options?: RequestOptionsArgs): Observable { return httpRequest( this._backend, new Request(mergeOptions(this._defaultOptions.merge(new RequestOptions({body: body})), @@ -159,7 +161,7 @@ export class Http { /** * Performs a request with `head` http method. */ - head(url: string, options?: RequestOptionsArgs): any { + head(url: string, options?: RequestOptionsArgs): Observable { return httpRequest(this._backend, new Request(mergeOptions(this._defaultOptions, options, RequestMethods.Head, url))); } @@ -177,7 +179,7 @@ export class Jsonp extends Http { * object can be provided as the 2nd argument. The options object will be merged with the values * of {@link BaseRequestOptions} before performing the request. */ - request(url: string | Request, options?: RequestOptionsArgs): any { + request(url: string | Request, options?: RequestOptionsArgs): Observable { var responseObservable: any; if (isString(url)) { url = new Request(mergeOptions(this._defaultOptions, options, RequestMethods.Get, url));