From 651279b21f550ad476e5c1138e7fda349efc7b8d Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Wed, 29 Apr 2015 15:55:58 +0100
Subject: [PATCH] log errors when downloading files and clean up failed
 downloads

---
 app/coffee/UrlFetcher.coffee            | 26 ++++++++++++++++++++++---
 test/unit/coffee/UrlFetcherTests.coffee |  4 +++-
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/app/coffee/UrlFetcher.coffee b/app/coffee/UrlFetcher.coffee
index 66c5edf..5082a15 100644
--- a/app/coffee/UrlFetcher.coffee
+++ b/app/coffee/UrlFetcher.coffee
@@ -1,15 +1,31 @@
 request = require("request").defaults(jar: false)
 fs = require("fs")
+logger = require "logger-sharelatex"
 
 module.exports = UrlFetcher =
 	pipeUrlToFile: (url, filePath, _callback = (error) ->) ->
 		callbackOnce = (error) ->
-			_callback(error)
-			_callback = () ->
+			cleanUp error, (error) ->
+				_callback(error)
+				_callback = () ->
+
+		cleanUp = (error, callback) ->
+			if error?
+				logger.log filePath: filePath, "deleting file from cache due to error"
+				fs.unlink filePath, (err) ->
+					if err?
+						logger.err err: err, filePath: filePath, "error deleting file from cache"
+					callback(error)
+			else
+				callback()
 
-		urlStream = request.get(url)
 		fileStream = fs.createWriteStream(filePath)
+		fileStream.on 'error', (error) ->
+			logger.error err: error, url:url, filePath: filePath, "error writing file into cache"
+			callbackOnce(error)
 
+		logger.log url:url, filePath: filePath, "downloading url to cache"
+		urlStream = request.get(url)
 		urlStream.on "response", (res) ->
 			if res.statusCode >= 200 and res.statusCode < 300
 				urlStream.pipe(fileStream)
@@ -17,7 +33,11 @@ module.exports = UrlFetcher =
 				callbackOnce(new Error("URL returned non-success status code: #{res.statusCode} #{url}"))
 
 		urlStream.on "error", (error) ->
+			logger.error err: error, url:url, filePath: filePath, "error downloading url"
 			callbackOnce(error or new Error("Something went wrong downloading the URL #{url}"))
 
 		urlStream.on "end", () ->
+			# FIXME: what if we get an error writing the file?  Maybe we
+			# should be using the fileStream end event as the point of
+			# callback.
 			callbackOnce()
diff --git a/test/unit/coffee/UrlFetcherTests.coffee b/test/unit/coffee/UrlFetcherTests.coffee
index 3e6dc92..7d38aa6 100644
--- a/test/unit/coffee/UrlFetcherTests.coffee
+++ b/test/unit/coffee/UrlFetcherTests.coffee
@@ -11,6 +11,7 @@ describe "UrlFetcher", ->
 		@UrlFetcher = SandboxedModule.require modulePath, requires:
 			request: defaults: @defaults = sinon.stub().returns(@request = {})
 			fs: @fs = {}
+			"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() }
 
 	it "should turn off the cookie jar in request", ->
 		@defaults.calledWith(jar: false)
@@ -21,7 +22,8 @@ describe "UrlFetcher", ->
 			@path = "/path/to/file/on/disk"
 			@request.get = sinon.stub().returns(@urlStream = new EventEmitter)
 			@urlStream.pipe = sinon.stub()
-			@fs.createWriteStream = sinon.stub().returns(@fileStream = "write-stream-stub")
+			@fs.createWriteStream = sinon.stub().returns(@fileStream = { on: () -> })
+			@fs.unlink = (file, callback) -> callback()
 			@UrlFetcher.pipeUrlToFile(@url, @path, @callback)
 
 		it "should request the URL", ->