Handle error cases when downloading URL

services/web/app/coffee/Features/LinkedFiles/LinkedFilesController.coffee

@@ -18,7 +18,8 @@ module.exports = LinkedFilesController = {
 		linkedFileData = Agent.sanitizeData(data)
 		linkedFileData.provider = provider
 		Agent.writeIncomingFileToDisk project_id, linkedFileData, user_id, (error, fsPath) ->
-			return next(error) if error?
+			if error?
+				return Agent.handleError(error, req, res, next)
 			EditorController.upsertFile project_id, parent_folder_id, name, fsPath, linkedFileData, "upload", user_id, (error) ->
 				return next(error) if error?
 				res.send(204) # created

services/web/app/coffee/Features/LinkedFiles/UrlAgent.coffee

@@ -1,17 +1,62 @@
 request = require 'request'
 FileWriter = require('../../infrastructure/FileWriter')
+_ = require "underscore"
+urlValidator = require 'valid-url'
+
+UrlFetchFailedError = (message) ->
+	error = new Error(message)
+	error.name = 'UrlFetchFailedError'
+	error.__proto__ = UrlFetchFailedError.prototype
+	return error
+UrlFetchFailedError.prototype.__proto__ = Error.prototype
+
+InvalidUrlError = (message) ->
+	error = new Error(message)
+	error.name = 'InvalidUrlError'
+	error.__proto__ = InvalidUrlError.prototype
+	return error
+InvalidUrlError.prototype.__proto__ = Error.prototype
+
 
 module.exports = UrlAgent = {
+	UrlFetchFailedError: UrlFetchFailedError
+	InvalidUrlError: InvalidUrlError
+
 	sanitizeData: (data) ->
 		return {
 			url: data.url
 		}
 
+	_prependHttpIfNeeded: (url) ->
+		if !url.match('://')
+			url = 'http://' + url
+		return url
+
 	writeIncomingFileToDisk: (project_id, data, current_user_id, callback = (error, fsPath) ->) ->
-		# TODO: Check it's a valid URL
 		# TODO: Proxy through external API
-		# TODO: Error unless valid status code
+		callback = _.once(callback)
-		url = data.url
+		url = @._prependHttpIfNeeded(data.url)
+		if !urlValidator.isWebUri(url)
+			return callback(new InvalidUrlError())
 		readStream = request.get(url)
-		FileWriter.writeStreamToDisk project_id, readStream, callback
+		readStream.on "error", callback
+		readStream.on "response", (response) ->
+			if 200 <= response.statusCode < 300
+				FileWriter.writeStreamToDisk project_id, readStream, callback
+			else
+				error = new UrlFetchFailedError()
+				error.statusCode = response.statusCode
+				callback(error)
+
+	handleError: (error, req, res, next) ->
+		if error instanceof UrlFetchFailedError
+			res.status(422).send(
+				"Your URL could not be reached (#{error.statusCode} status code). Please check it and try again."
+			)
+		else if error instanceof InvalidUrlError
+			res.status(422).send(
+				"Your URL is not valid. Please check it and try again."
+			)
+		else
+			next(error)
 }

services/web/package.json

@@ -85,6 +85,7 @@
     "underscore": "1.6.0",
     "uuid": "^3.0.1",
     "v8-profiler": "^5.2.3",
+    "valid-url": "^1.0.9",
     "xml2js": "0.2.0",
     "yauzl": "^2.8.0"
   },

services/web/test/acceptance/coffee/LinkedFilesTests.coffee

@@ -1,5 +1,6 @@
 async = require "async"
 expect = require("chai").expect
+_ = require 'underscore'
 
 MockFileStoreApi = require './helpers/MockFileStoreApi'
 MockURLSource	= require './helpers/MockURLSource'
@@ -92,3 +93,87 @@ describe "LinkedFiles", ->
 							expect(response.statusCode).to.equal 200
 							expect(body).to.equal "bar bar bar"
 							done()
+
+		it "should return an error if the URL does not succeed", (done) ->
+			@owner.request.post {
+				url: "/project/#{@project_id}/linked_file",
+				json:
+					provider: 'url'
+					data: {
+						url: "http://localhost:6543/does-not-exist"
+					}
+					parent_folder_id: @root_folder_id
+					name: 'url-test-file-3'
+			}, (error, response, body) =>
+				throw error if error?
+				expect(response.statusCode).to.equal 422 # unprocessable
+				expect(body).to.equal(
+					"Your URL could not be reached (404 status code). Please check it and try again."
+				)
+				done()
+
+		it "should return an error if the URL is invalid", (done) ->
+			@owner.request.post {
+				url: "/project/#{@project_id}/linked_file",
+				json:
+					provider: 'url'
+					data: {
+						url: "!^$%"
+					}
+					parent_folder_id: @root_folder_id
+					name: 'url-test-file-4'
+			}, (error, response, body) =>
+				throw error if error?
+				expect(response.statusCode).to.equal 422 # unprocessable
+				expect(body).to.equal(
+					"Your URL is not valid. Please check it and try again."
+				)
+				done()
+
+		it "should return an error if the URL uses a non-http protocol", (done) ->
+			@owner.request.post {
+				url: "/project/#{@project_id}/linked_file",
+				json:
+					provider: 'url'
+					data: {
+						url: "ftp://localhost"
+					}
+					parent_folder_id: @root_folder_id
+					name: 'url-test-file-5'
+			}, (error, response, body) =>
+				throw error if error?
+				expect(response.statusCode).to.equal 422 # unprocessable
+				expect(body).to.equal(
+					"Your URL is not valid. Please check it and try again."
+				)
+				done()
+
+		it "should accept a URL withuot a leading http://, and add it", (done) ->
+			@owner.request.post {
+				url: "/project/#{@project_id}/linked_file",
+				json:
+					provider: 'url'
+					data: {
+						url: "http://localhost:6543/foo"
+					}
+					parent_folder_id: @root_folder_id
+					name: 'url-test-file-6'
+			}, (error, response, body) =>
+				throw error if error?
+				expect(response.statusCode).to.equal 204
+				@owner.getProject @project_id, (error, project) =>
+					throw error if error?
+					file = _.find project.rootFolder[0].fileRefs, (file) ->
+						file.name == 'url-test-file-6'
+					expect(file.linkedFileData).to.deep.equal({
+						provider: 'url'
+						url: "http://localhost:6543/foo"
+					})
+					@owner.request.get "/project/#{@project_id}/file/#{file._id}", (error, response, body) ->
+						throw error if error?
+						expect(response.statusCode).to.equal 200
+						expect(body).to.equal "foo foo foo"
+						done()
+
+		# TODO: Add test for asking for host that return ENOTFOUND
+		# (This will probably end up handled by the proxy)