airbnb / hypernova

A service for server-side rendering your JavaScript views
MIT License
5.82k stars 207 forks source link

Proper source map support for development mode #145

Open kpelelis opened 5 years ago

kpelelis commented 5 years ago

Hello. In order to speed up our development process, we have build a custom solution for the hypernova server which can load Javascript bundles over HTTP. With that, we can spin up a webpack dev server and reload the compiled bundle each time the source files change. That has been working wonders for the time being and we are really happy that we don't have to compile the files over and over again.

In order to make development even better, we wanted to add source map support for the generated stack traces. Our current implementation is the following.

1) Client asks for component Foo 2) We load the code for component Foo over the webpack dev server 3) We evaluate its code with vm.runInNewContext and we catch any errors generated 4) Should we catch any error, we load the source map file and we feed it into a sourceMapConsumer from Mozilla's source-map. 5) We then map all the stack trace elements into new ones by querying the consumer. 6) Finally we throw the modified error.

Code would look something like this

let scriptContent;
  try {
    const response = await axios.get(url);
    scriptContent = response.data;
  } catch (e) {
    console.error('Error while getting remote file');
    console.error(e);
    throw e;
  }
  let module = require('module');

  const wrappedCode = module.wrap(scriptContent);
  const moduleRunner = vm.runInNewContext(wrappedCode, createContext(), {
    displayErrors: true,
    filename: 'ssr.node.js'
  });

  const { data } = await axios.get(`${url}.map`)
  const sourceMapConsumer = await new sourceMap.SourceMapConsumer(data);

  return () => {
    const modExports = {};
    const modContext = {};

    try {
      moduleRunner(modExports, require, modContext, __filename, __dirname);
    } catch(error) {
      // Get the trace lines
      const trace = error.stack.split("\n").slice(1).map(l => l.trim());
      const originalStackTrace = sourceMappedStackTrace(trace, sourceMapConsumer);
      // Map them to the original files
      // Construct the new stack trace
      const newTrace = originalStackTrace.map((orig, index) => {
        if(orig.source === null || orig.line === null || orig.column === null) {
          // Something that we don't have a mapping for. Leave the original
          return `    ${trace[index]}`;
        }
        let base = '    at ';
        if(orig.name) {
          base += `${orig.name} `;
        }
        base += `${orig.source}:${orig.line}:${orig.column}`;
        return base;
      })
      .filter(e => e !== null)

      // Throw the modified error
      throw new Error(error.stack.split('\n')[0] + '\n' + newTrace.join('\n'))
    }
    return modContext.exports;

Although this works great for the time being, we wanted to catch "runtime" errors as well. That is, errors generated when (e.g. ReactDOM.renderToString runs). To my understanding, there is no way to manipulate such errors from BatchManager.render as the exceptions are caught and modified there.

We could submit a patch upstream to enable custom error handling if there is no other elegant way of manipulating error traces.

ljharb commented 5 years ago

I would expect that your build process would include a source map url inline in the code, which would be fetched as needed by the browser and updated natively as your code hot reloads, without needing to do any hijacking of errors.

kpelelis commented 5 years ago

Problem is, the bundle will never reach the browser:

--> Browser requests page (e.g. /home) --> Hypernova Ruby Client (needs to partially render a div with hypernova, so it requests hypernova for a render) --> Hypernova (tries to render the component and gets and error, so it returns the error) --> Hypernova Ruby Client (renders the error generated from nova) --> Browser renders the HTML page generated

Indeed we have source map generated (with webpack)

ljharb commented 5 years ago

Gotcha - and in this case, the client render doesn’t take over?

kpelelis commented 5 years ago

If by take over, you mean handling the stack trace the answer is no. The Ruby Client knows nothing of the original javascript bundle that the error came from. The idea was to transform the source map in the hypernova server

agis commented 5 years ago

@ljharb For example, the backtrace displayed on the browser is something along the lines of:

"ReferenceError: idontexist is not defined",
     "at new SampleComponent (ssr.bundle.js:35348:5)",
     "at processChild (ssr.bundle.js:34431:14)",
     "at resolve (ssr.bundle.js:34397:5)",
     "at ReactDOMServerRenderer.render (ssr.bundle.js:34724:22)",
     "at ReactDOMServerRenderer.read (ssr.bundle.js:34695:25)",
     "at Object.renderToString (ssr.bundle.js:35109:25)",
     "at ssr.bundle.js:11566:46",
     "at /home/dev/node_modules/hypernova/lib/utils/BatchManager.js:190:18",
     "at tryCatcher (/home/dev/node_modules/bluebird/js/release/util.js:16:23)",
     "at Promise._settlePromiseFromHandler (/home/dev/node_modules/bluebird/js/release/promise.js:512:31)"

Note that:

Could it be that rolling our own implementation (with vm.runInNewContext()) instead of using createGetComponent is messing up the source maps?

Thanks!

ljharb commented 5 years ago

Gotcha. So you do have a source map for that bundle, you just want it somehow applied in the hypernova error.

That does seem like something that could be done inside hypernova itself. However, perhaps if you included https://www.npmjs.com/package/source-map-support yourself, it would be handled for you?

kpelelis commented 5 years ago

Yeah! We already tried that by pre-pending the snippet required by source-map-support in the bundle with the help of webpack. Problem is, runInNewContext messes up the line offset so the source map is sort of invalid.

We finally got a solution going with a rather hacky way. Instead of using a webpack dev server and loading the module over http in hypernova server, we run a normal webpack server with —watch and we load the bundle from the file system. In this way we can employ all the cool features of hypernova without any custom code. We still need to pre-pend the snippet though, but it works just fine.

If it is okay with you, we can patch the server so that a user, or a plugin can handle errors.

In any case, thanks a lot for looking into it.

Cheers, Skroutz team

ljharb commented 5 years ago

It's hard for me to really understand the scope of this without a PR. Would you be willing to make one, with the understanding that there's no promise to merge it? If it's reasonable and doesn't seem to add much complexity, I'm sure it'd be fine, but I want to set your expectations appropriately :-)