Skip to content

Commit e59c814

Browse files
author
Arthur Cinader
committed
Log Parse Errors so they are intelligible.
The problem this pr is trying to solve: When an error occurs on the server, a message should be returned to the client, and a message should be logged. Currently, on the server, the log is just [object, object] This pr will stop calling the default express error handler which causes two problems: 1. it writes to console instead of log file 2. the output is completely useless! :) Instead, we'll log the error ourselves using the ParseServer's logger. fixes: #661
1 parent 43c3d8e commit e59c814

File tree

3 files changed

+53
-19
lines changed

3 files changed

+53
-19
lines changed

spec/CloudCodeLogger.spec.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ describe("Cloud Code Logger", () => {
194194
Parse.Cloud.run('aFunction', { foo: 'bar' })
195195
.then(null, () => logController.getLogs({ from: Date.now() - 500, size: 1000 }))
196196
.then(logs => {
197-
const log = logs[1];
197+
const log = logs[2];
198198
expect(log.level).toEqual('error');
199199
expect(log.message).toMatch(
200200
/Failed running cloud function aFunction for user [^ ]* with:\n {2}Input: {"foo":"bar"}\n {2}Error: {"code":141,"message":"it failed!"}/);

spec/FilesController.spec.js

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,17 @@
1-
var GridStoreAdapter = require("../src/Adapters/Files/GridStoreAdapter").GridStoreAdapter;
2-
var Config = require("../src/Config");
3-
var FilesController = require('../src/Controllers/FilesController').default;
1+
const LoggerController = require('../src/Controllers/LoggerController').LoggerController;
2+
const WinstonLoggerAdapter = require('../src/Adapters/Logger/WinstonLoggerAdapter').WinstonLoggerAdapter;
3+
const GridStoreAdapter = require("../src/Adapters/Files/GridStoreAdapter").GridStoreAdapter;
4+
const Config = require("../src/Config");
5+
const FilesController = require('../src/Controllers/FilesController').default;
46

7+
const mockAdapter = {
8+
createFile: () => {
9+
return Parse.Promise.reject(new Error('it failed'));
10+
},
11+
deleteFile: () => { },
12+
getFileData: () => { },
13+
getFileLocation: () => 'xyz'
14+
}
515

616
// Small additional tests to improve overall coverage
717
describe("FilesController",() =>{
@@ -26,5 +36,25 @@ describe("FilesController",() =>{
2636
expect(anObject.aFile.url).toEqual("http://an.url");
2737

2838
done();
29-
})
39+
});
40+
41+
it('should create a server log on failure', done => {
42+
const logController = new LoggerController(new WinstonLoggerAdapter());
43+
44+
reconfigureServer({ filesAdapter: mockAdapter })
45+
.then(() => new Promise(resolve => setTimeout(resolve, 1000)))
46+
.then(() => new Parse.File("yolo.txt", [1,2,3], "text/plain").save())
47+
.then(
48+
() => done.fail('should not succeed'),
49+
() => setImmediate(() => Parse.Promise.as('done'))
50+
)
51+
.then(() => logController.getLogs({ from: Date.now() - 500, size: 1000 }))
52+
.then((logs) => {
53+
const log = logs.pop();
54+
expect(log.level).toBe('error');
55+
expect(log.code).toBe(130);
56+
expect(log.message).toBe('Could not store file.');
57+
done();
58+
});
59+
});
3060
});

src/middlewares.js

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
import AppCache from './cache';
2-
import log from './logger';
3-
import Parse from 'parse/node';
4-
import auth from './Auth';
5-
import Config from './Config';
6-
import ClientSDK from './ClientSDK';
1+
import AppCache from './cache';
2+
import log from './logger';
3+
import Parse from 'parse/node';
4+
import auth from './Auth';
5+
import Config from './Config';
6+
import ClientSDK from './ClientSDK';
77

88
// Checks that the request is authorized for this app and checks user
99
// auth too.
@@ -233,10 +233,8 @@ export function allowMethodOverride(req, res, next) {
233233
}
234234

235235
export function handleParseErrors(err, req, res, next) {
236-
// TODO: Add logging as those errors won't make it to the PromiseRouter
237236
if (err instanceof Parse.Error) {
238-
var httpStatus;
239-
237+
let httpStatus;
240238
// TODO: fill out this mapping
241239
switch (err.code) {
242240
case Parse.Error.INTERNAL_SERVER_ERROR:
@@ -250,17 +248,23 @@ export function handleParseErrors(err, req, res, next) {
250248
}
251249

252250
res.status(httpStatus);
253-
res.json({code: err.code, error: err.message});
251+
res.json({ code: err.code, error: err.message });
252+
log.error(err.message, err);
254253
} else if (err.status && err.message) {
254+
log.error(err.message);
255255
res.status(err.status);
256-
res.json({error: err.message});
256+
res.json({ error: err.message });
257+
next(err);
257258
} else {
258259
log.error('Uncaught internal server error.', err, err.stack);
259260
res.status(500);
260-
res.json({code: Parse.Error.INTERNAL_SERVER_ERROR,
261-
message: 'Internal server error.'});
261+
res.json({
262+
code: Parse.Error.INTERNAL_SERVER_ERROR,
263+
message: 'Internal server error.'
264+
});
265+
next(err);
262266
}
263-
next(err);
267+
264268
}
265269

266270
export function enforceMasterKeyAccess(req, res, next) {

0 commit comments

Comments
 (0)