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

fix($http): JSON parse failure #15724

Closed
wants to merge 6 commits into from
Closed

fix($http): JSON parse failure #15724

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
68ced16
fix($http): JSON parse failure
chirag64 Feb 19, 2017
573e0da
fix($http): JSON parse failure
chirag64 Feb 20, 2017
9d0c8b8
Merge branch 'master' of https://github.com/angular/angular.js into j…
chirag64 Feb 21, 2017
7deaae9
fix($http): JSON parse failure
chirag64 Feb 21, 2017
28cc76c
Merge branch 'master' of https://github.com/angular/angular.js into j…
chirag64 Mar 4, 2017
91393a2
fix($http): JSON parse failure
chirag64 Mar 4, 2017
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
21 changes: 4 additions & 17 deletions test/ng/httpSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1369,17 +1369,15 @@ describe('$http', function() {
}
);

it('should forward json deserialization errors to the http error handler',
function() {
it('should return JSON data with error message if JSON is invalid', function() {
var errCallback = jasmine.createSpy('error');

$httpBackend.expect('GET', '/url').respond('abcd', {'Content-Type': 'application/json'});
$http({method: 'GET', url: '/url'}).then(callback).catch(errCallback);
$httpBackend.expect('GET', '/url').respond('{abcd}', {'Content-Type': 'application/json'});
Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've replaced the it function of forwarding json deserialization errors with mine. The above highlighted statements are pretty much similar to the old code except that we're using an object '{abcd}' within the string instead of a direct string 'abcd'. Are you referring to that string or something else?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fine with me. @gkalpak wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

@chirag64, yes, I am referring to the string. Why change abcd to {abcd}?
I am mostly curious - I don't think it makes any difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to make it look clear that we're passing an invalid JSON instead of passing a string.

$http.get('/url').then(callback).catch(errCallback);
$httpBackend.flush();

expect(callback).not.toHaveBeenCalled();
expect(errCallback).toHaveBeenCalledOnce();
expect(errCallback.calls.mostRecent().args[0]).toEqual(jasmine.any(SyntaxError));
expect(errCallback.calls.mostRecent().args[0]).toEqualMinErr('$http', 'baddata');
});

});
Expand Down Expand Up @@ -1442,17 +1440,6 @@ describe('$http', function() {
expect(callback.calls.argsFor(1)[0]).toBe(null);
expect(callback.calls.argsFor(2)[0]).toBe('');
});

it('should return JSON data with error message if JSON is invalid', function() {
var errCallback = jasmine.createSpy('error');
$httpBackend.expect('GET', '/url').respond('{abcd}', {'Content-Type': 'application/json'});
$http.get('/url').then(callback).catch(errCallback);
$httpBackend.flush();

expect(callback).not.toHaveBeenCalled();
expect(errCallback).toHaveBeenCalledOnce();
expect(errCallback.calls.mostRecent().args[0]).toEqualMinErr('$http', 'baddata');
});
});
});

Expand Down