Merge pull request #488 from sharelatex/bg-404-for-missing-doc

return 404 for api request on missing doc

services/web/app/coffee/Features/Docstore/DocstoreManager.coffee

@@ -1,6 +1,7 @@
 request = require("request").defaults(jar: false)
 logger = require "logger-sharelatex"
 settings = require "settings-sharelatex"
+Errors  = require "../Errors/Errors"
 
 module.exports = DocstoreManager =
 	deleteDoc: (project_id, doc_id, callback = (error) ->) ->
@@ -10,6 +11,10 @@ module.exports = DocstoreManager =
 			return callback(error) if error?
 			if 200 <= res.statusCode < 300
 				callback(null)
+			else if res.statusCode is 404
+				error = new Errors.NotFoundError("tried to delete doc not in docstore")
+				logger.error err: error, project_id: project_id, doc_id: doc_id, "tried to delete doc not in docstore"
+				callback(error) # maybe suppress the error when delete doc which is not present?
 			else
 				error = new Error("docstore api responded with non-success code: #{res.statusCode}")
 				logger.error err: error, project_id: project_id, doc_id: doc_id, "error deleting doc in docstore"
@@ -61,6 +66,10 @@ module.exports = DocstoreManager =
 			if 200 <= res.statusCode < 300
 				logger.log doc_id: doc_id, project_id: project_id, version: doc.version, rev: doc.rev, "got doc from docstore api"
 				callback(null, doc.lines, doc.rev, doc.version, doc.ranges)
+			else if res.statusCode is 404
+				error = new Errors.NotFoundError("doc not found in docstore")
+				logger.error err: error, project_id: project_id, doc_id: doc_id, "doc not found in docstore"
+				callback(error)
 			else
 				error = new Error("docstore api responded with non-success code: #{res.statusCode}")
 				logger.error err: error, project_id: project_id, doc_id: doc_id, "error getting doc from docstore"

services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee

@@ -3,6 +3,7 @@ chai.should()
 sinon = require("sinon")
 modulePath = "../../../../app/js/Features/Docstore/DocstoreManager"
 SandboxedModule = require('sandboxed-module')
+Errors = require "../../../../app/js/Features/Errors/Errors.js"
 
 describe "DocstoreManager", ->
 	beforeEach ->
@@ -52,6 +53,23 @@ describe "DocstoreManager", ->
 					}, "error deleting doc in docstore")
 					.should.equal true
 
+		describe "with a missing (404) response code", ->
+			beforeEach ->
+				@request.del = sinon.stub().callsArgWith(1, null, statusCode: 404, "")
+				@DocstoreManager.deleteDoc @project_id, @doc_id, @callback
+
+			it "should call the callback with an error", ->
+				@callback.calledWith(new Errors.NotFoundError("tried to delete doc not in docstore")).should.equal true
+
+			it "should log the error", ->
+				@logger.error
+					.calledWith({
+						err: new Errors.NotFoundError("tried to delete doc not in docstore")
+						project_id: @project_id
+						doc_id: @doc_id
+					}, "tried to delete doc not in docstore")
+					.should.equal true
+
 	describe "updateDoc", ->
 		beforeEach ->
 			@lines = ["mock", "doc", "lines"]
@@ -153,6 +171,23 @@ describe "DocstoreManager", ->
 			it "should call the callback with the lines, version and rev", ->
 				@callback.calledWith(null, @lines, @rev, @version, @ranges).should.equal true
 
+		describe "with a missing (404) response code", ->
+			beforeEach ->
+				@request.get = sinon.stub().callsArgWith(1, null, statusCode: 404, "")
+				@DocstoreManager.getDoc @project_id, @doc_id, @callback
+
+			it "should call the callback with an error", ->
+				@callback.calledWith(new Errors.NotFoundError("doc not found in docstore")).should.equal true
+
+			it "should log the error", ->
+				@logger.error
+					.calledWith({
+						err: new Errors.NotFoundError("doc not found in docstore")
+						project_id: @project_id
+						doc_id: @doc_id
+					}, "doc not found in docstore")
+					.should.equal true
+
 	describe "getAllDocs", ->
 		describe "with a successful response code", ->
 			beforeEach ->