rubyjs / mini_racer

Minimal embedded v8
MIT License
598 stars 93 forks source link

Meaningful errors with inlined sourcemaps #58

Open siassaj opened 7 years ago

siassaj commented 7 years ago

Hi

I'm using webpack to compile a server side js bundle with inlined sourcemaps, used to render server side views. Everything's going well except errors. The stack trace given back is reasonable, eg:

ExecJS::ProgramError - ReferenceError: func is not defined:
  Widget.vtree (eval at <anonymous> ((execjs):357:in `'
  Widget.render (eval at <anonymous> ((execjs):391:in `'
  Object.render (eval at <anonymous> ((execjs):1379:in `'

This error is okay, i can follow it from the entry point but I don't get a strong gauge on which line in which file, which would be available in chrome, for example.

Is there a way to do that, to get more specific information about which line and which file is causing the exception?

ignisf commented 7 years ago

Hi,

Could you by any chance provide a sample app we can work off of?

Thanks :)

SamSaffron commented 7 years ago

Resolving source maps is an adventure, we would have to parse them and build special handling for them, I guess eval could accept a map as a param so it is technically feasable but a huge job to implement.

I think the best way to go about this would be to make another gem for source map resolution which if included with mini racer provides the option.

On Fri, Jun 2, 2017 at 3:20 PM, Petko Bordjukov notifications@github.com wrote:

Hi,

Could you by any chance provide a sample app we can work off of?

Thanks :)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/discourse/mini_racer/issues/58#issuecomment-305886271, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAUXemHaCXx4bxh0VGXpUcQ-Mp5neBLks5sAGCFgaJpZM4Nup14 .

ignisf commented 7 years ago

I wanted to try if I could do something with https://github.com/evanw/node-source-map-support/. Do you think it could be useful in this case?

siassaj commented 7 years ago

@SamSaffron your comment, it seems to imply that V8 isn't going to change how it reports it's errors, and that the consumer (in this case mini_racer) would have to scan the sourcemap and construct a meaningful view of the error itself from the data returned by V8.

Is that a fair assessment?

siassaj commented 7 years ago

@ignisf tried to make that work but it was futile

SamSaffron commented 7 years ago

V8 does not implement any source map logic, the logic all exists in the inspector which is a javascript package from what I can tell.

On Fri, Jun 2, 2017 at 10:54 PM, siassaj notifications@github.com wrote:

@SamSaffron https://github.com/samsaffron your comment, it seems to imply that V8 isn't going to change how it reports it's errors, and that the consumer (in this case mini_racer) would have to scan the sourcemap and construct a meaningful view of the error itself from the data returned by V8.

Is that a fair assessment?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/discourse/mini_racer/issues/58#issuecomment-305946071, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAUXQd9Zxx1uyOkXusObM9uoUdHNXKhks5sAMrVgaJpZM4Nup14 .

tisba commented 7 years ago

Can you elaborate a bit what was the biggest problem when you gave https://github.com/evanw/node-source-map-support a try, @siassaj?

SamSaffron commented 7 years ago

I just committed and deployed filename support for MiniRacer.

But... sourcemap resolution all happens after that. You have to run your backtrace through a parser, load files and resolve locations, its a tedious job.

siassaj commented 7 years ago

Hi all @tisba sorry didn't see the github notification. I will read into it soon and see what's up :)

@SamSaffron to clarify, you mean that whatever process runs miniracer should collect the error and the backtrace, then run another program/routine that uses the sourcemap and source files to figure out the correct code to show, then send it back to the original program to show the user, yes?

SamSaffron commented 7 years ago

to clarify, you mean that whatever process runs miniracer should collect the error and the backtrace, then run another program/routine that uses the sourcemap and source files to figure out the correct code to show, then send it back to the original program to show the user, yes?

Yes, something like that.

In fact all the code that does this kind of magic is already written for node, you could probably just use mini racer to run the lookup and so on.

tisba commented 7 years ago

I think that maybe using https://github.com/mozilla/source-map directly could do the trick. I started to play around with it a little bit, but I haven't progressed very far yet…

SamSaffron commented 7 years ago

cool @tisba let us know how you go! I would be very happy to include this kind of functionality in mini racer, but it is a biggish project.

ianks commented 5 years ago

Source maps are working well for me now. here's what i did:

ctx = MiniRacer::Context.new
ctx.eval(my_script_string, filename: "my_script_string.js") # important!
ctx.attach('readSourceMap', proc { |source_filename| File.read("#{source_filename].map") })

now, somewhere in your js:

if (process.env.NODE_ENV === "production") {
  require("source-map-support").install({
    retrieveSourceMap: (filename) => {
      return {
        url: filename,
        // @ts-ignore
        map: readSourceMap(filename)
      };
    }
  });
}

This works with a fairly advanced webpack setup flawlessly.

SamSaffron commented 5 years ago

I love this example, maybe do a PR to add it to the README @ianks ? example just needs to be fleshed up

tisba commented 5 years ago

@ianks could you provide a more complete example? Or are you running on an unreleased version of mini racer? Because last time I checked require is not a feature provided by mini racer. Using your example, I get ReferenceError: require is not defined as I would expect.

Unless support for source maps was added to mini racer, I'm a bit confused about how backtraces would get translated 😕

ianks commented 5 years ago

hey @tisba, this example uses webpack to load the package https://www.npmjs.com/package/source-map-support. i'll clean this example up and get it in the readme

ianks commented 5 years ago

@tisba the source-map-support packages deals with mapping the traces, and it works since miniracer supports the filename option when eval'ing.

tisba commented 5 years ago

-duh- okay, sorry that makes perfect sense. The require is translated by webpack and the npm package will patch Error.prototype (or something like that).

So technically no webpack is required and you could simply "eval" source-map-support, attach readSourceMap and then eval your script - at least if I understand this correctly.