vazco / universe-modules

Use ES6 / ES2015 modules in Meteor with SystemJS
https://atmospherejs.com/universe/modules
MIT License
52 stars 7 forks source link

No server-side error reporting on loaded modules #35

Closed amirmohsen closed 8 years ago

amirmohsen commented 8 years ago

First off, thank you for making this package. It's been a life-saver. Organizing a large meteor project without it is an absolute nightmare. I think you should try and approach the MDG group to see if they are willing to incorporate it into core. It's absolutely essential to Meteor's scalability as it grows in popularity. If you could add lazy-loading to this package, then it would be particularly awesome!

Anyway, I've come across a rather serious issue when using this package on server files. It seems the package has somehow disabled Meteor's error reporting on server modules. So instead of a stack trace in the console, all I get is the server code silently failing which is quite dangerous in production-level code.

I give you an example:

I've put my deny/allow rules in separate files for each collection, then I manually load each one from the entry point. So for instance, in one file, I have:

import Blocks from "path/to/BlocksCollection";

// Obviously, the following is not the actual rules.
Blocks.allow({
    insert: () => {
        return true;
    },

    update: () => {
        return true;
    },

    delete: () => {
        return true;
    }
});

export default {};

Now, in another file, I am importing all these rules:

import BlocksCollectionPermissions from "path/to/BlocksCollectionPermissions";
// More import statements here
// ...

You may have noticed that in my rules, I've used the delete keyword, by mistake, instead of remove. In a normal .js file, I immediately get an error thrown in the console and the code doesn't compile. Unfortunately, in an import.js module, this isn't the case. I don't get any errors and server code fails silently.

Hope the example gets my point across. I'm making heavy use of this package in a production application now. Please can you look into this? It seems like a serious issue. Thank you.

MacRusher commented 8 years ago

Thank you for kind words :) MDG is actually working on modules and it should land in 1.3: https://github.com/meteor/meteor/pull/5475 We will try to provide migration path when this happen, but unfortunately their solution differs from ours in some base principles (SystemJS vs webpack-alike)

About your issue, its probably related to promises.

When you're loading modules by System.import your code is executed asynchronously in a promise. When error occurs its not propagated up to main program but instead catched by a promise.

Apparently in MGD promise implementation that modules are using there is a global catch for all uncaught errors on the client, but the same isn't true for server side.

Solution for your problem is to catch errors on System.import, e.g.:

System.import('/moduleName').then(module => {
    // everything ok
}).catch(err => console.error(err));

There used to be a default catch on import in previous version of universe:modules for a short time, but it behave quite unstable and produce double error notification on client side. Maybe we can work this out in future version.

Let me know if adding a catch statement solves your problem.

rozzzly commented 8 years ago

I also ran into this issue, but came up with a nice solution.

I'm using [vazco/meteor-universe-modules-entrypoint] but that shouldn't make a differences. In my entrypoint file, main.js, I use the System.import(...) syntax to load my initial imports. This way I can force code from server/ to load before both/. In the rest of the app, I use the regular es6 import x from y; syntax. I just import server/server.entrypoint.js and everything is taken care of for me!

note to self: eventually I'm going to have to redesign this a little so not everything needs to be loaded before it can start running. I'm not noticing any performance issues, but once my app gets larger... huge? I'll have selectively load things.

Like @MacRusher suggested, System.import returns a promise so you can do .then(...) and .catch(...).

I'll try to find the link later today, but I found an issue on this repo with the same question you have. the same question I had. The solution is a bit hacky, but take a look:

if (Meteor.isServer) {
    System.import('{}/server/server.entrypoint').then((_server) => {
        console.log('main.js  -- server.entrypoint.then()');
        System.import('{}/both/both.entrypoint').then((_both) => {
            console.log('entrypoint.both.then() callback from entrypoint.server.then()');
        }).catch((err) => {
            setTimeout(() => {
                errText('caught by server/both.import');
                throw err;
            });
        });
    }).catch((err) => {
        setTimeout(() => {
            errText('caught by server.import');
            throw err;
        });
    });
} else if (Meteor.isClient) {
    // EXACT same code except replace 'server' with 'client'
}

example stack trace!

It works like it normally should! With how @MacRusher suggested, which was what I was doing at first, I only just a tiny error message. Meteor is to blame for needing the setTimeOut(...), I forget why exactly but I'll reference the issue explaining when I find it.

W20151118-13:57:23.613(-5)? (STDERR) Error when doing SSR. path: /finesse/button   Invariant Violation: ButtonDemo.render(): A valid ReactComponent must be returned. You may have returned undefined, an array or some other invalid object.
W20151118-13:57:24.230(-5)? (STDERR) Error: Invariant Violation: ButtonDemo.render(): A valid ReactComponent must be returned. You may have returned undefined, an array or some other invalid object.
W20151118-13:57:24.231(-5)? (STDERR)     at invariant (node_modules/react/node_modules/fbjs/lib/invariant.js:39:1)
W20151118-13:57:24.231(-5)? (STDERR)     at [object Object].ReactCompositeComponentMixin._renderValidatedComponent (../../../../../../../var/www/html/tom/OnlineServices/app/react/lib/ReactCompositeComponent:613:1)
W20151118-13:57:24.232(-5)? (STDERR)     at [object Object].wrapper [as _renderValidatedComponent] (../../../../../../../var/www/html/tom/OnlineServices/app/react/lib/ReactPerf:66:1)
W20151118-13:57:24.232(-5)? (STDERR)     at [object Object].ReactCompositeComponentMixin.mountComponent (../../../../../../../var/www/html/tom/OnlineServices/app/react/lib/ReactCompositeComponent:220:1)
W20151118-13:57:24.232(-5)? (STDERR)     at [object Object].wrapper [as mountComponent] (../../../../../../../var/www/html/tom/OnlineServices/app/react/lib/ReactPerf:66:1)
W20151118-13:57:24.232(-5)? (STDERR)     at Object.ReactReconciler.mountComponent (../../../../../../../var/www/html/tom/OnlineServices/app/react/lib/ReactReconciler:37:1)
W20151118-13:57:24.233(-5)? (STDERR)     at ReactDOMComponent.ReactMultiChild.Mixin.mountChildren (../../../../../../../var/www/html/tom/OnlineServices/app/react/lib/ReactMultiChild:241:1)
W20151118-13:57:24.233(-5)? (STDERR)     at ReactDOMComponent.Mixin._createContentMarkup (../../../../../../../var/www/html/tom/OnlineServices/app/react/lib/ReactDOMComponent:588:1)
W20151118-13:57:24.233(-5)? (STDERR)     at ReactDOMComponent.Mixin.mountComponent (../../../../../../../var/www/html/tom/OnlineServices/app/react/lib/ReactDOMComponent:478:1)
W20151118-13:57:24.234(-5)? (STDERR)     at Object.ReactReconciler.mountComponent (../../../../../../../var/www/html/tom/OnlineServices/app/react/lib/ReactReconciler:37:1)

as a bonus, because I'm finicky I also have this in there

function errText(customMessage = '') {
    console.error(`
8 88888888888  8 888888888o.   8 888888888o.      ,o888888o.     8 888888888o.
8 8888         8 8888    \`88.  8 8888    \`88.  . 8888     \`88.   8 8888    \`88.
8 8888         8 8888     \`88  8 8888     \`88 ,8 8888       \`8b  8 8888     \`88
8 8888         8 8888     ,88  8 8888     ,88 88 8888        \`8b 8 8888     ,88
8 888888888888 8 8888.   ,88'  8 8888.   ,88' 88 8888         88 8 8888.   ,88'
8 8888         8 888888888P'   8 888888888P'  88 8888         88 8 888888888P'
8 8888         8 8888\`8b       8 8888\`8b      88 8888        ,8P 8 8888\`8b
8 8888         8 8888 \`8b.     8 8888 \`8b.    \`8 8888       ,8P  8 8888 \`8b.
8 8888         8 8888   \`8b.   8 8888   \`8b.   \` 8888     ,88'   8 8888   \`8b.
8 888888888888 8 8888     \`88. 8 8888     \`88.    \`8888888P'     8 8888     \`88.

    ${customMessage}`);
}
amirmohsen commented 8 years ago

Thanks guys. The solution you offered does indeed work. Looking forward to Meteor's own module system. Closing the issue now.