Log Parse Errors so they are intelligible. (#3431)
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
This commit is contained in:
committed by
Florent Vilmart
parent
711db9ccd2
commit
f864141663
@@ -194,7 +194,7 @@ describe("Cloud Code Logger", () => {
|
||||
Parse.Cloud.run('aFunction', { foo: 'bar' })
|
||||
.then(null, () => logController.getLogs({ from: Date.now() - 500, size: 1000 }))
|
||||
.then(logs => {
|
||||
const log = logs[1];
|
||||
const log = logs[2];
|
||||
expect(log.level).toEqual('error');
|
||||
expect(log.message).toMatch(
|
||||
/Failed running cloud function aFunction for user [^ ]* with:\n {2}Input: {"foo":"bar"}\n {2}Error: {"code":141,"message":"it failed!"}/);
|
||||
|
||||
@@ -1,7 +1,17 @@
|
||||
var GridStoreAdapter = require("../src/Adapters/Files/GridStoreAdapter").GridStoreAdapter;
|
||||
var Config = require("../src/Config");
|
||||
var FilesController = require('../src/Controllers/FilesController').default;
|
||||
const LoggerController = require('../src/Controllers/LoggerController').LoggerController;
|
||||
const WinstonLoggerAdapter = require('../src/Adapters/Logger/WinstonLoggerAdapter').WinstonLoggerAdapter;
|
||||
const GridStoreAdapter = require("../src/Adapters/Files/GridStoreAdapter").GridStoreAdapter;
|
||||
const Config = require("../src/Config");
|
||||
const FilesController = require('../src/Controllers/FilesController').default;
|
||||
|
||||
const mockAdapter = {
|
||||
createFile: () => {
|
||||
return Parse.Promise.reject(new Error('it failed'));
|
||||
},
|
||||
deleteFile: () => { },
|
||||
getFileData: () => { },
|
||||
getFileLocation: () => 'xyz'
|
||||
}
|
||||
|
||||
// Small additional tests to improve overall coverage
|
||||
describe("FilesController",() =>{
|
||||
@@ -26,5 +36,25 @@ describe("FilesController",() =>{
|
||||
expect(anObject.aFile.url).toEqual("http://an.url");
|
||||
|
||||
done();
|
||||
})
|
||||
});
|
||||
|
||||
it('should create a server log on failure', done => {
|
||||
const logController = new LoggerController(new WinstonLoggerAdapter());
|
||||
|
||||
reconfigureServer({ filesAdapter: mockAdapter })
|
||||
.then(() => new Promise(resolve => setTimeout(resolve, 1000)))
|
||||
.then(() => new Parse.File("yolo.txt", [1,2,3], "text/plain").save())
|
||||
.then(
|
||||
() => done.fail('should not succeed'),
|
||||
() => setImmediate(() => Parse.Promise.as('done'))
|
||||
)
|
||||
.then(() => logController.getLogs({ from: Date.now() - 500, size: 1000 }))
|
||||
.then((logs) => {
|
||||
const log = logs.pop();
|
||||
expect(log.level).toBe('error');
|
||||
expect(log.code).toBe(130);
|
||||
expect(log.message).toBe('Could not store file.');
|
||||
done();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
import AppCache from './cache';
|
||||
import log from './logger';
|
||||
import Parse from 'parse/node';
|
||||
import auth from './Auth';
|
||||
import Config from './Config';
|
||||
import ClientSDK from './ClientSDK';
|
||||
import AppCache from './cache';
|
||||
import log from './logger';
|
||||
import Parse from 'parse/node';
|
||||
import auth from './Auth';
|
||||
import Config from './Config';
|
||||
import ClientSDK from './ClientSDK';
|
||||
|
||||
// Checks that the request is authorized for this app and checks user
|
||||
// auth too.
|
||||
@@ -233,10 +233,8 @@ export function allowMethodOverride(req, res, next) {
|
||||
}
|
||||
|
||||
export function handleParseErrors(err, req, res, next) {
|
||||
// TODO: Add logging as those errors won't make it to the PromiseRouter
|
||||
if (err instanceof Parse.Error) {
|
||||
var httpStatus;
|
||||
|
||||
let httpStatus;
|
||||
// TODO: fill out this mapping
|
||||
switch (err.code) {
|
||||
case Parse.Error.INTERNAL_SERVER_ERROR:
|
||||
@@ -250,17 +248,22 @@ export function handleParseErrors(err, req, res, next) {
|
||||
}
|
||||
|
||||
res.status(httpStatus);
|
||||
res.json({code: err.code, error: err.message});
|
||||
res.json({ code: err.code, error: err.message });
|
||||
log.error(err.message, err);
|
||||
} else if (err.status && err.message) {
|
||||
res.status(err.status);
|
||||
res.json({error: err.message});
|
||||
res.json({ error: err.message });
|
||||
next(err);
|
||||
} else {
|
||||
log.error('Uncaught internal server error.', err, err.stack);
|
||||
res.status(500);
|
||||
res.json({code: Parse.Error.INTERNAL_SERVER_ERROR,
|
||||
message: 'Internal server error.'});
|
||||
res.json({
|
||||
code: Parse.Error.INTERNAL_SERVER_ERROR,
|
||||
message: 'Internal server error.'
|
||||
});
|
||||
next(err);
|
||||
}
|
||||
next(err);
|
||||
|
||||
}
|
||||
|
||||
export function enforceMasterKeyAccess(req, res, next) {
|
||||
|
||||
Reference in New Issue
Block a user