404 results in a fatal error

[dtjm]

Hi, first of all thank you for writing this tool. I've been wanting to make something like this for a while.

I am trying to pull down this web page but one of the links resolves to a 404 error, which halts processing of the page. Would you be open to introducing a flag that ignores the 404 error and leaves the link as-is?

I would be glad to try to make a pull request, but would need some hints as to where to start.

Thanks!

[remy]

It makes sense to not bail on 404/by default actually. The code change is in find-asset.js where it throws if the status is not 200.

On Wed, 26 Aug 2015 06:10 Sam Nguyen notifications@github.com wrote:

Hi, first of all thank you for writing this tool. I've been wanting to make something like this for a while.

I am trying to pull down this web page http://danquyen.com/?cmd=act:news%7Cnewsid:17931 but one of the links resolves to a 404 error, which halts processing of the page. Would you be open to introducing a flag that ignores the 404 error and leaves the link as-is?

I would be glad to try to make a pull request, but would need some hints as to where to start.

Thanks!

— Reply to this email directly or view it on GitHub https://github.com/remy/inliner/issues/61.

[remy]

Sorry, just to add if you want a crack, please do. It should be straight forward, but one of the current PRs tries to address it but uses fallback content which I'm not quite on board with (and also checkout the contributing doc as it'll explain how to add tests - which is easy!).

On Wed, 26 Aug 2015 07:29 Remy Sharp remy@remysharp.com wrote:

It makes sense to not bail on 404/by default actually. The code change is in find-asset.js where it throws if the status is not 200.

On Wed, 26 Aug 2015 06:10 Sam Nguyen notifications@github.com wrote:

Hi, first of all thank you for writing this tool. I've been wanting to make something like this for a while.

I am trying to pull down this web page http://danquyen.com/?cmd=act:news%7Cnewsid:17931 but one of the links resolves to a 404 error, which halts processing of the page. Would you be open to introducing a flag that ignores the 404 error and leaves the link as-is?

I would be glad to try to make a pull request, but would need some hints as to where to start.

Thanks!

— Reply to this email directly or view it on GitHub https://github.com/remy/inliner/issues/61.

[dtjm]

I'm not seeing a check for 200 in https://github.com/remy/inliner/blob/master/lib/find-assets.js, although I do see it here: https://github.com/remy/inliner/blob/master/lib/get.js#L58

However, I'm trying to figure what do to there. I'm thinking of something like this:

      if (error || (res && res.statusCode != 200)) {
          resolve({ url: url });
      } else {
          resolve({ headers: res.headers, body: body });
      }

but I can't figure out what code is consuming that promise.

[remy]

Oh, you're right - that's attached to inliner.get and reused across all the asset collection.

So, I think that body should probably be a blank string...and then resolve. I think it's also worth triggering a warning on the progress event, via:

inliner.emit('progress', res.statusCode + ' on ' + url);