Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Errors in lazy loaded components are not propagated and have no source map support #1798

Closed
nemtsov opened this issue Oct 10, 2017 · 15 comments

Comments

@nemtsov
Copy link

nemtsov commented Oct 10, 2017

Version

2.7.0

Reproduction link

https://codesandbox.io/s/176k2vp03

Steps to reproduce

  1. Run the codesandbox.io link.
  2. Notice that the component isn't rendered.
  3. Look at the console to see the unhandled error.
  4. In SSR and on the browser, this error isn't propagated to the usual error handler. In SSR I tested with renderToString
  5. In SSR the error does not use source maps and is very difficult to debug.

More info

When using VUE Router Lazy Loading, and there's an error outside of the component, the error is not propagated & the stack trace isn't cleaned up with source maps. Here's the gist of the minimal reproduction (linked):

e.g. MyComponent.vue

<script>
throw new Error('bad_error');

export default {
  name: 'MyComponent'
};
</script>

createRouter.js

const MyComponent = () => import('./MyComponent.vue');

export default createRouter() {
  return new Router({
    routes: [
      { path: '/', component: MyComponent }
    ]
  });
}

Error in the logs:

[vue-router] Failed to resolve async component default: Error: bad_error
[vue-router] uncaught error during route navigation:
 Error: bad_error
     at Object.exports.modules.Array.concat.Object.defineProperty.value (0.server-bundle.js:12842:7)
     at __webpack_require__ (server-bundle.js:27:30)
    at Object.<anonymous> (0.server-bundle.js:12708:141)
     at __webpack_require__ (server-bundle.js:27:30)
     at Object.exports.modules.Array.concat.Object.de

What is expected?

I expect the error to be propagated to the renderToString err in the callback and the error to be rewritten using the source maps.

What is actually happening?

When rendered with SSR, the page hangs, as the error is never propagated to renderToString's err in the callback.

@Austio
Copy link

Austio commented Oct 10, 2017

Timely, we ran into this today as well.

@posva
Copy link
Member

posva commented Oct 11, 2017

I don't see why you would have an error raised outside the component code but inside of the component file. If it's raised inside, you can see the actual code for the component and Vue is able to catch it but otherwise, it cannot.
The router may be able to catch it but why are you throwing errors on your component module?
You can get around it by catching it btw:

const Hello = () => import('../components/Hello')
.catch((err) => ({
     render: h => h('div', [
          h('h1', err.name),
          h('pre', err.stacktrace)
    ])
}))

(or similar)

@nemtsov
Copy link
Author

nemtsov commented Oct 11, 2017

@posva the error thrown in my example represents an unexpected error; I should have clarified that part. When we import libraries or other components it's not unusual to have code between the imports and the exported code. Sometimes this is done for caching, other times for ergonomics. That code gets executed when the module is imported. Sometimes there are unexpected errors in that code.

@posva
Copy link
Member

posva commented Oct 11, 2017

When does that happen? By accident when developing?

@nemtsov
Copy link
Author

nemtsov commented Oct 11, 2017

Examples of modules I've seen importing which will sometimes throw:

import fs from 'fs';

const MY_DATA = fs.readFileSync('./myFile.csv');

export default {
  getUsers() {
    // use MY_DATA
  }
};
...

The example above will throw on import if myFile.cvs does not exist and will otherwise pass.


const HASH = window.btoa('asdf')

export default {
  getSomething() {
    // use HASH
  }
};
...

The example above will throw when using the code in a WebWorker or in Node.js. It will work as expected in the browser.


The point I'm trying to make is that it's not just typos during development. There are bugs in the logic that could throw on import and there are missed conditions or environments that can make a module throw on import.

@posva
Copy link
Member

posva commented Oct 11, 2017

To be fair, you should catch errors of the readFileSync call and you're responsible to make sure your code runs correctly in places it should.
In any case, these problems appear clearly in the console and you can always return a fallback component in the catch as shown. It's the kind of error you would get rid during development
screen shot 2017-10-11 at 15 41 34
It would, indeed, be a problem if the errors were not reported, but they are

@posva posva closed this as completed Oct 11, 2017
@mikesherov
Copy link

First, let me say thanks for the excellent work here. We love Vue and vue-router and none of this is possible without the hard work and dedication of people like you.

To be fair, you should catch errors of the readFileSync call and you're responsible to make sure your code runs correctly in places it should.

This is definitely true, with an emphasis on should. The point of this issue is the case where you unfortunately haven’t, or otherwise made a mistake. People make mistakes in both dev and Prod, and having clear stack traces in Prod when you’ve unfortunately missed a catch or errBack somewhere is even more critical.

Please reconsider closing this issue.

@posva
Copy link
Member

posva commented Oct 11, 2017

If a module throws when imported, it will throw in dev, otherwise, it will be during component's life and therefore caught by Vue. In any case, you should be able to get better stack traces if you have sourcemaps

@Austio
Copy link

Austio commented Oct 11, 2017

On our end this occurred during Server Side Rendering, someone included a polyfill that was not node compatible that was being included in an imported component.

@mikesherov
Copy link

There are situations where it doesn't fail in dev where it may in PROD: fixtures / flaky network / testing only happy paths, etc etc. There are many ways in which a module does something on import that can fail in an unexpected way, and the developer in dev didn't catch it, and only starts failing in prod at like 4AM Saturday morning, exactly when you most dearly need those clear stack traces.

If there is nothing vue-router can do here to help that, or if this is considered an unlikely scenario, I get that. But it definitely does happen.

@nemtsov
Copy link
Author

nemtsov commented Oct 11, 2017

To echo @mikesherov: the work that you're doing on Vue @posva is amazing and is greatly appreciated. This library, like others in the Vue ecosystem is reliable and a pleasure to work with because of the work that you do.

I now see how confusing this issue is without a complete and proper repro; which I'm working on and will post here. The issue that motivated me to report this is with this code in SSR. Using renderToString(app, (err, html) => you expect either to get html or to get an err. When you get an err (provided you have source-maps enabled) the err stack is cleaned up to show the actual line-numbers, file and function names.

The issues we are seeing are:

  1. The err in renderToString does not get called when there's an import-time error, neither does the callback. This is really bad because there's no way to handle such an error gracefully. Ultimately, either the browser times out or the user gives up and closes the tab.
  2. The second issue with this is that if you look in the server logs, the error is there, but there's no way to trace to the file that threw it because the source-maps are not used to clean up the error.

Once again, I'll provide a minimal repro with SSR for this.

@Austio
Copy link

Austio commented Oct 11, 2017

@nemtsov this was the polyfill that was added if you need something that will easily blow up in node :) https://github.com/jimmywarting/FormData

@posva
Copy link
Member

posva commented Oct 11, 2017

I'll wait for the ssr repro @nemtsov but I'm not sure if we'll be able to do something in vue-router or even vue. We'll see

@nemtsov
Copy link
Author

nemtsov commented Oct 11, 2017

Ok. I figured out the first of the two issues, where the callback of renderToString is not being called.

The behavior that was unclear to me is that (a) router.onReady() takes two arguments where the second one is the error callback and that (b) a rejection from a lazy component doesn't call the router.onReady()'s callback, but instead the error-callback and the router.onError() (which I wasn't aware of).

Take a look at the repro here:
https://runkit.com/nemtsov/vue-ssr-component-routing-onerror

Note the

// test('/good')
// test('/render_caught')
test('/router_caught')

If you uncomment the "/good" only, you'll see the onReady called and the component rendered. If you uncomment only the /render_caught you'll also see that the onReady is called and the component rendered. And if you uncomment the last one (/router_caught), the onReady doesn't get caught and instead the onError gets called.

So, the first one is not an issue. The second one (re source-maps) I think I'll move to vue's issues, as I don't think it has to do with vue-router.

Thank you @posva


Edit: I've created a full repro for the second of the two issues here: vuejs/vue#6778

@posva
Copy link
Member

posva commented Oct 12, 2017

Thanks for the concise repro 🙂
I really don't know what to do for the sourcemaps there. I'll let someone else chime in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants