add handling for image returned with 404 http response (fixes #399)

[wsdwsd0829]

There are cases when image is returned along with 404 HttpResponse, this pr will handle the edge case from within framework.
Since we manage cache any way, may be not necessary to expose api to client to handle this case?
Side Notes: NSUrlSession can handle this case correctly in NSURLSessionDataDelegate.

[wsdwsd0829]

@garrettmoon let me know if you have any ideas around it, Thanks.

[ghost]

🚫 CI failed with log

[wsdwsd0829]

CI failing because of flakiness of testResume & testResumeSkipCancelation

[wsdwsd0829]

@appleguy @garrettmoon new fixes based on the fact that url session works for 404 image out of box.

[appleguy]

@garrettmoon - it looks like testResume might be a flaky test; should it be disabled?

PINRemoteImage_Tests
testResume, ((resume.resumeData.length > 0) is true) failed - Resume should have > 0 data length
/Users/Shared/buildkite/builds/iosf-garrett-VMrmmvuqKsO4.dyn.pinadmin.com-1/pinterest/pinremoteimage/Tests/PINRemoteImageTests.m:1169

  PINResume *resume = [self.imageManager.cache objectFromDiskForKey:[self.imageManager resumeCacheKeyForURL:[self progressiveURL]]];
  XCTAssert(resume.resumeData.length > 0, @"Resume should have > 0 data length");

testResume, failed: caught "NSInternalInconsistencyException", "PINResume must have all fields non-nil and non-zero length."
/Users/Shared/buildkite/builds/iosf-garrett-VMrmmvuqKsO4.dyn.pinadmin.com-1/pinterest/pinremoteimage/Tests/PINRemoteImageTests.m:1172

  //Shorten resume data to improve reliability of test (expect to get progressive render callback before download completes.
  resume = [PINResume resumeData:[resume.resumeData subdataWithRange:NSMakeRange(0, 10)] ifRange:resume.ifRange totalBytes:resume.totalBytes];
  [self.imageManager.cache setObjectOnDisk:resume forKey:[self.imageManager resumeCacheKeyForURL:[self progressiveURL]]];

[ghost]

🚫 CI failed with log

[wsdwsd0829]

This fixes #399, similar issues reported here SDWebImage/SDWebImage#530

[garrettmoon]

@appleguy sadly it's totally flakey, but I'm not willing to disable at this point. I am however happy to rerun the tests as many times as necessary :)

I've actually spent a bit of time attempting to make the tests less flakey, but I think the only solution is to write a web server for the tests and I'm reluctant to invest in that right now :(

[ghost]

🚫 CI failed with log

[garrettmoon]

Should the result include an error if there's a 404 as well as an image? Otherwise how can the client of PINRemoteImage know a 404 was returned?

Separately, is it common practice to return an image with a 404 response? Any documentation on something like this?

[garrettmoon]

Can you use the same timeout as used in the other tests?

[wsdwsd0829]

fixed

[garrettmoon]

cough

[ghost]

🚫 CI failed with log

[wsdwsd0829]

If we return the error, the PINRemoteImageManager will not handler data correctly (the error will short cut the data to image handling, can be see my previous attempt to fix here 539137c). Also I thought this is a very special case and responseRecoverableFrom404 can handle it well enough.
If it's just normal 404 there would not be data and "image/jpeg" in response header;

[wsdwsd0829]

examples refer to here: SDWebImage/SDWebImage#530
curl https://i4.ytimg.com -i

[ghost]

🚫 CI failed with log

[garrettmoon]

Do you mind adding a comment that says something along the lines of:
"If a 404 response contains an image, we treat it as a successful request and return the image"

[wsdwsd0829]

added & thanks for approval.

[ghost]

🚫 CI failed with log

[ghost]

🚫 CI failed with log

[ghost]

🚫 CI failed with log

[maicki]

LGTM

[garrettmoon]

YAY! CI Passed!

@wsdwsd0829 do you mind adding an entry to the CHANGELOG.md?

[wsdwsd0829]

Added to change log