Merge pull request #189 from overleaf/jpa-relevel-log

[misc] re-level expected error log messages
This commit is contained in:
Jakob Ackermann 2020-09-02 10:46:08 +02:00 committed by GitHub
commit c6f2a3b387
6 changed files with 182 additions and 1 deletions

View file

@ -33,6 +33,7 @@ module.exports = Router = {
}
attrs.client_id = client.id
attrs.err = error
attrs.method = method
if (error.name === 'CodedError') {
logger.warn(attrs, error.message)
const serializedError = { message: error.message, code: error.info.code }

View file

@ -8,6 +8,7 @@ const logger = require('logger-sharelatex')
const {
CodedError,
CorruptedJoinProjectResponseError,
NotAuthorizedError,
WebApiRequestFailedError
} = require('./Errors')
@ -55,6 +56,10 @@ module.exports = {
'TooManyRequests'
)
)
} else if (response.statusCode === 403) {
callback(new NotAuthorizedError())
} else if (response.statusCode === 404) {
callback(new CodedError('project not found', 'ProjectNotFound'))
} else {
callback(new WebApiRequestFailedError(response.statusCode))
}

View file

@ -356,7 +356,7 @@ module.exports = WebsocketController = {
cursorData.doc_id,
function (error) {
if (error) {
logger.warn(
logger.info(
{ err: error, client_id: client.id, project_id, user_id },
"silently ignoring unauthorized updateClientPosition. Client likely hasn't called joinProject yet."
)

View file

@ -176,6 +176,128 @@ describe('joinProject', function () {
})
})
describe('when not authorized and web replies with a 403', function () {
before(function (done) {
return async.series(
[
(cb) => {
return FixturesManager.setUpProject(
{
project_id: 'forbidden',
privilegeLevel: 'owner',
project: {
name: 'Test Project'
}
},
(e, { project_id, user_id }) => {
this.project_id = project_id
this.user_id = user_id
cb(e)
}
)
},
(cb) => {
this.client = RealTimeClient.connect()
this.client.on('connectionAccepted', cb)
},
(cb) => {
this.client.emit(
'joinProject',
{ project_id: this.project_id },
(error, project, privilegeLevel, protocolVersion) => {
this.error = error
this.project = project
this.privilegeLevel = privilegeLevel
this.protocolVersion = protocolVersion
cb()
}
)
}
],
done
)
})
it('should return an error', function () {
this.error.message.should.equal('not authorized')
})
it('should not have joined the project room', function (done) {
RealTimeClient.getConnectedClient(
this.client.socket.sessionid,
(error, client) => {
expect(Array.from(client.rooms).includes(this.project_id)).to.equal(
false
)
done()
}
)
})
})
describe('when deleted and web replies with a 404', function () {
before(function (done) {
return async.series(
[
(cb) => {
return FixturesManager.setUpProject(
{
project_id: 'not-found',
privilegeLevel: 'owner',
project: {
name: 'Test Project'
}
},
(e, { project_id, user_id }) => {
this.project_id = project_id
this.user_id = user_id
cb(e)
}
)
},
(cb) => {
this.client = RealTimeClient.connect()
this.client.on('connectionAccepted', cb)
},
(cb) => {
this.client.emit(
'joinProject',
{ project_id: this.project_id },
(error, project, privilegeLevel, protocolVersion) => {
this.error = error
this.project = project
this.privilegeLevel = privilegeLevel
this.protocolVersion = protocolVersion
cb()
}
)
}
],
done
)
})
it('should return an error', function () {
this.error.code.should.equal('ProjectNotFound')
})
it('should not have joined the project room', function (done) {
RealTimeClient.getConnectedClient(
this.client.socket.sessionid,
(error, client) => {
expect(Array.from(client.rooms).includes(this.project_id)).to.equal(
false
)
done()
}
)
})
})
return describe('when over rate limit', function () {
before(function (done) {
return async.series(

View file

@ -38,6 +38,12 @@ module.exports = MockWebServer = {
joinProjectRequest(req, res, next) {
const { project_id } = req.params
const { user_id } = req.query
if (project_id === 'not-found') {
return res.status(404).send()
}
if (project_id === 'forbidden') {
return res.status(403).send()
}
if (project_id === 'rate-limited') {
return res.status(429).send()
} else {

View file

@ -91,6 +91,53 @@ describe('WebApiManager', function () {
})
})
describe('when web replies with a 403', function () {
beforeEach(function () {
this.request.post = sinon
.stub()
.callsArgWith(1, null, { statusCode: 403 }, null)
this.WebApiManager.joinProject(
this.project_id,
this.user_id,
this.callback
)
})
it('should call the callback with an error', function () {
this.callback
.calledWith(
sinon.match({
message: 'not authorized'
})
)
.should.equal(true)
})
})
describe('when web replies with a 404', function () {
beforeEach(function () {
this.request.post = sinon
.stub()
.callsArgWith(1, null, { statusCode: 404 }, null)
this.WebApiManager.joinProject(
this.project_id,
this.user_id,
this.callback
)
})
it('should call the callback with an error', function () {
this.callback
.calledWith(
sinon.match({
message: 'project not found',
info: { code: 'ProjectNotFound' }
})
)
.should.equal(true)
})
})
describe('with an error from web', function () {
beforeEach(function () {
this.request.post = sinon