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

Cannot put breakpoints in dash.debug.js. Source map mismatch? #977

Closed
TobbeMobiTV opened this issue Jan 5, 2016 · 17 comments
Closed

Cannot put breakpoints in dash.debug.js. Source map mismatch? #977

TobbeMobiTV opened this issue Jan 5, 2016 · 17 comments

Comments

@TobbeMobiTV
Copy link
Contributor

I cannot put proper breakpoints in the dash.debug.js file in the dev version of 2.0.

When trying to put a breakpoint in dash.debug.js, I always end up with a breakpoint on line 43 in Error.js, independently on where I click in the dash.debug.js file.

Commit: c1ecff2
App: Reference Player:
Browser: Chrome 47

This looks like a source map mismatch to me. I tried to turn off mangle in for uglify in Gruntfile.js, but it didn't make any difference.

@boushley
Copy link
Member

boushley commented Jan 5, 2016

Definitely something we should look into.

As a temporary workaround you could run grunt build and then copy the file from build/temp/Dash.all.js to be the version you're working with. This should avoid the uglify minimization entirely (although all the files will still be concat'ed together).

I'm unsure what the commit you've linked to has to do with this issue though.

@TobbeMobiTV
Copy link
Contributor Author

I've tried to use Dash.all.js, but I get to "random" positions when trying to set breakpoints in that case as well. Maybe there is some issue when concatenating the files since the external files have not been processed in the same way as the internal...

The commit I listed has nothing particular to do with the problem.

@boushley
Copy link
Member

boushley commented Jan 5, 2016

Are you using Dash.all.js from running grunt build instead of running grunt dist? If you just run grunt build there shouldn't be any source maps in play. It will be the single concat'ed file with no source map.

Even without a source map I've run into this problem sometimes (although not with this library) and it appears to be a Chrome issue where it doesn't quite detect that the lines are breakable. I've gotten significantly improved results by hitting the auto format button in Chrome. That seems to let Chrome know that it can break on more of the lines. Wonder what we can do to mitigate this problem.

@TobbeMobiTV
Copy link
Contributor Author

I don't have any external source map files in this case, but as far as I understand browserify runs babel which changes the code and since debug is set to true, it creates embedded source maps. These maps are later extracted by exorcise when minified is run.

In any case, something seems to be broken.

@boushley
Copy link
Member

boushley commented Jan 5, 2016

@TobbeMobiTV you're right. I missed the sourceMapURL comments in the debug version. Sorry for the confusion.

@boushley
Copy link
Member

boushley commented Jan 5, 2016

I'm able to reproduce the error you're seeing, most of the dash.debug.js file ends up dropping a breakpoint at the bottom of Error.js.

Some more info for future debugging for me I get dropped into prelude.js if I click towards the top of the file, and I had one that dropped me (correctly) into FactoryMaker.js. Although the majority of lines seem to drop me into Error.js

The formatted source workaround does seem to work for me though. I don't get the source files, but if I click the {} button in Chrome I can set breakpoint accordingly inside of the formatted dash.debug.js certainly not the ideal, but should help debug while we get the source maps sorted out.

@boushley
Copy link
Member

boushley commented Jan 5, 2016

Alright, I think I've tracked down the underlying issue.

We're running exorcise on Dash.all.js but Dash.all.js has two source map entries in it. Exorcism is just pulling out the last match in this case. So the Dash.all.js.map file only maps to the second piece of the file. We need to get concat to merge these source maps or eliminate concat entirely and let uglify do the concatenation of files.

@TobbeMobiTV
Copy link
Contributor Author

@boushley That makes sense. Thanks for finding the root cause.

It works for me if I don't concatenate MediaPlayer.js and Protection.js and just concatenate all externals into a third file that I include.

I'm not sure what the right thing is for the project, but it seems to me that one must do early concatenation before the source map is generated.

@boushley
Copy link
Member

boushley commented Jan 5, 2016

@TobbeMobiTV I've got the same thing working locally.

Trying to figure out the best way to concat these files and still produce a valid source map. grunt-concat-sourcemap looks promising but is having some issues. I'll keep you posted.

@boushley
Copy link
Member

boushley commented Jan 5, 2016

So I've made a bit more progress. It looks like combining source maps can be a fools errand when you've got as many tools in play as we do. So I've taken a different approach. Ran into another bug, it looks like uglify generates a bad source map if you use the beautify option. Luckily the source maps seem to work fine when compressing and if you have the source maps then the beautified version is less important.

I'll try and refine this down to a good PR.

@boushley
Copy link
Member

boushley commented Jan 5, 2016

It'll be hard to test my PR until we find a resolution to #980

@dsparacio
Copy link
Contributor

@boushley @TobbeMobiTV - I believe i mentioned this in my email before i left but not sure anymore. Source maps are sometimes failing due to all the things you discoverd in this thread! But when you say breakpoints in dash.debug.js are not working, did you delete the .map file before trying to do this?

If I deleted the dash.debug.js map I could debug in the debug.js file and actually was my solution for when the actualy source map to es6 code failed. .

There are a few other problems with certain files and it all has to do with concat and the external files being added in.... i thinks.

@boushley
Copy link
Member

boushley commented Jan 9, 2016

@AkamaiDASH hope you had a good vacation!

I think I've got a build working locally where we end up with a dash.all.min.js with a functional map file. The map file works for everything except the external libs. Do we think that's good enough? What I have is better than the current state, but I can keep digging to try and figure out how to get source maps for the external files too if we want.

The main fix for min was to concat the external files on after we've uglified our source. That way the source map is fully functional and won't be changed again once we add on the externals. That gave me the best results. Although until we get #980 resolved I can't really test it very thoroughly since the min version of the files doesn't work.

@dsparacio
Copy link
Contributor

@boushley - First thank you had a great vaction and ready to get going again. Also thanks for looking at this and other items.

Just to be clear when you say "The map file works for everything except the external libs." From what I am reading you concat externals after the minification and exorcising. So we still get the single "all" file with externals but the map is based on just dash.js code. I think this will work fine for now. We could always use the min file and include each external file if we need to debug them.

We need to talk about names for all the variants now... This will be a topic in tomorrow's meeting. Let me look at 980 today and respond to that thread.

@boushley
Copy link
Member

@AkamaiDASH you understood correctly. That is what I have functional locally (still a single all file, externals don't get source mapped).

I actually don't use exorcise anymore in my local branch.

It would be great if I could at least listen in on tomorrow's phone call. If you can send me the details that would be excellent. My email is my Github username @gmail.com

@wilaw
Copy link
Member

wilaw commented Jan 12, 2016

@aaron and anyone else who is interested – the call dial-in details (and the minutes of all the calls for the past two years if you are a masochist) are available here: https://github.com/Dash-Industry-Forum/dash.js/wiki/Meeting-Minutes

The next one will be tomorrow at 10am PST. We will have a call the following week on the 19th and every 2 weeks thereafter at our regular cadence. You need to create your own recurring calendar invite as google groups strips out ones that I try to send.

Cheers

Will

From: Aaron Boushley
Reply-To: "Dash-Industry-Forum/dash.js"
Date: Monday, January 11, 2016 at 3:56 PM
To: "Dash-Industry-Forum/dash.js"
Subject: Re: [dash.js] Cannot put breakpoints in dash.debug.js. Source map mismatch? (#977)

@AkamaiDASHhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_AkamaiDASH&d=CwMCaQ&c=96ZbZZcaMF4w0F4jpN6LZg&r=KkevKJerDHRF9WRs8nW8Ew&m=MUallkri2AmJFr9_dVB3jj6S_pavlol_EBHIsC7OBuU&s=7D5y8HSEw1MBvVPx-L94wKN4Lo04BCl05yHPFZ1FNfY&e= you understood correctly. That is what I have functional locally (still a single all file, externals don't get source mapped).

I actually don't use exorcise anymore in my local branch.

It would be great if I could at least listen in on tomorrow's phone call. If you can send me the details that would be excellent. My email is my Github username @gmail.com


Reply to this email directly or view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Dash-2DIndustry-2DForum_dash.js_issues_977-23issuecomment-2D170736538&d=CwMCaQ&c=96ZbZZcaMF4w0F4jpN6LZg&r=KkevKJerDHRF9WRs8nW8Ew&m=MUallkri2AmJFr9_dVB3jj6S_pavlol_EBHIsC7OBuU&s=_7OwZHReZX8TGM_bmdd6qrVMli7aqtnLeFQpxRGkV54&e=.

@dsparacio dsparacio added this to the 2.0.0 milestone Jan 12, 2016
boushley added a commit to boushley/dash.js that referenced this issue Jan 12, 2016
This produces sourcemap files that are valid for dash.js code.

When running uglify with the beautify parameter set to true, some sourcemaps are corrupted.
For an example you can enable maps for .debug and add a breakpoint in Stream.js setup function
which results in a breakpoint in the BufferController onCurrentTrackChanged function.
boushley added a commit to boushley/dash.js that referenced this issue Jan 12, 2016
This produces sourcemap files that are valid for dash.js code.

When running uglify with the beautify parameter set to true, some sourcemaps are corrupted.
For an example you can enable maps for .debug and add a breakpoint in Stream.js setup function
which results in a breakpoint in the BufferController onCurrentTrackChanged function.
dsparacio pushed a commit that referenced this issue Jan 12, 2016
@dsparacio
Copy link
Contributor

Fixed closing

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

No branches or pull requests

4 participants