Skip to content

Commit

Permalink
Improve error handling for circular error objs
Browse files Browse the repository at this point in the history
  • Loading branch information
larixer committed Nov 29, 2017
1 parent 0e8676f commit 9d28329
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 17 deletions.
8 changes: 7 additions & 1 deletion src/common/log.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,15 @@ if (__DEV__ && __SERVER__) {
if (arguments.length == 1 && typeof arguments[0] === 'string' && arguments[0].match(/^\[(HMR|WDS)\]/)) {
console_log('backend ' + arguments[0]);
} else {
console_log.apply(console_log, arguments);
console_log.apply(global.console, arguments);
}
};

// let console_err = global.console.error;
// global.console.error = function() {
// arguments[0] = 'ce ' + new Error().stack + '\n\n\n' + arguments[0];
// console_err.apply(global.console, arguments);
// };

This comment has been minimized.

Copy link
@mairh

mairh Nov 29, 2017

Member

Why do we need dead code?

This comment has been minimized.

Copy link
@larixer

larixer Nov 29, 2017

Author Member

I have left it here, because it is quite common, when some 3rd party tool is polluting console with its errors. And you have no other means then intercept it like this. This way I has figured out that apollo-server is working in debug mode by default and pollutes console with GraphQL errors, which is brain damaged behaviour and you have no other way then interception like in this code to figure this out

This comment has been minimized.

Copy link
@mairh

mairh Nov 29, 2017

Member

Ok. Maybe a comment on top of it would explain this behavior nicely. At the moment, I don't think people will have any idea (including me) regarding the commented out piece of code.

This comment has been minimized.

Copy link
@larixer

larixer Nov 29, 2017

Author Member

@mairh Okay, I think I will remove this dead code

}

export default log;
5 changes: 1 addition & 4 deletions src/server/api/schema.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
import { makeExecutableSchema, addErrorLoggingToSchema } from 'graphql-tools';
import { makeExecutableSchema } from 'graphql-tools';

import rootSchemaDef from './rootSchema.graphqls';
import modules from '../modules';
import pubsub from './pubsub';
import log from '../../common/log';

const executableSchema = makeExecutableSchema({
typeDefs: [rootSchemaDef].concat(modules.schemas),
resolvers: modules.createResolvers(pubsub)
});

addErrorLoggingToSchema(executableSchema, { log: e => log.error(e) });

export default executableSchema;
20 changes: 16 additions & 4 deletions src/server/middleware/error.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,25 @@
import path from 'path';
import fs from 'fs';
import serialize from 'serialize-javascript';
import log from '../../common/log';
import { options as spinConfig } from '../../../.spinrc.json';

let assetMap;

const stripCircular = (from, seen) => {
const to = Array.isArray(from) ? [] : {};
seen = seen || [];
seen.push(from);
Object.getOwnPropertyNames(from).forEach(key => {
if (!from[key] || (typeof from[key] !== 'object' && !Array.isArray(from[key]))) {
to[key] = from[key];
} else if (seen.indexOf(from[key]) < 0) {
to[key] = stripCircular(from[key], seen.slice(0));
} else to[key] = '[Circular]';
});
return to;
};

/*
* The code below MUST be declared as a function, not closure,
* otherwise Express will fail to execute this handler
Expand All @@ -16,10 +31,7 @@ function errorMiddleware(e, req, res, next) {
assetMap = JSON.parse(fs.readFileSync(path.join(spinConfig.frontendBuildDir, 'web', 'assets.json')));
}

const serverErrorScript = `<script charset="UTF-8">window.__SERVER_ERROR__=${JSON.stringify(
e,
Object.getOwnPropertyNames(e)
)};</script>`;
const serverErrorScript = `<script charset="UTF-8">window.__SERVER_ERROR__=${serialize(stripCircular(e))};</script>`;
const vendorScript = assetMap['vendor.js'] ? `<script src="/${assetMap['vendor.js']}" charSet="utf-8"></script>` : '';

res.status(200).send(
Expand Down
5 changes: 3 additions & 2 deletions src/server/middleware/graphql.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ export default graphqlExpress(async (req, res, next) => {
return {
schema,
context: await modules.createContext(req, res),
tracing: !!settings.analytics.apolloEngine.key,
cacheControl: !!settings.analytics.apolloEngine.key
debug: false,
tracing: !!settings.engine.engineConfig.apiKey,
cacheControl: !!settings.engine.engineConfig.apiKey
};
} catch (e) {
next(e);
Expand Down
8 changes: 2 additions & 6 deletions src/server/middleware/website.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,8 @@ const renderServerSide = async (req, res) => {

const fetch = createApolloFetch({ uri: apiUrl, constructOptions: modules.constructFetchOptions });
fetch.batchUse(({ options }, next) => {
try {
options.credentials = 'include';
options.headers = req.headers;
} catch (e) {
console.error(e);
}
options.credentials = 'include';
options.headers = req.headers;

next();
});
Expand Down

0 comments on commit 9d28329

Please sign in to comment.