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
Changes from 2 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
21 changes: 4 additions & 17 deletions test/ng/httpSpec.js
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