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

integrated debugging #109

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

integrated debugging #109

wants to merge 4 commits into from

Conversation

dwickern
Copy link
Collaborator

@dwickern dwickern commented Nov 23, 2016

This change associates source files with URLs so that IntelliJ can find your sources.

intellij debugging

  • only supports "classic" structure (no pods)
  • supports in-repo addons
  • probably can't debug addons
  • only supports baseUrl: '/'
  • Requires ES6->ES5 sourcemaps, otherwise breakpoints will not match the same line as the source code; see Enable ES6 sourcemaps ember-cli/ember-cli#6458

Debugging steps:

  1. From the menu select Run -> Edit Configurations...
  2. Click the + button and select "JavaScript Debug"
  3. Fill out the URL to your Ember app, e.g. http://localhost:4200
  4. Click the green bug to launch the debugger

Without this change, it's possible to debug by mapping /app to http://localhost:4200/assets/my-app. However you can't have two mappings for the same URL, so you can't debug in-repo addons.

@Turbo87
Copy link
Owner

Turbo87 commented Nov 23, 2016

that is awesome!! 😍

@dwickern
Copy link
Collaborator Author

I tried debugging from IntelliJ for a bit, it works quite well.

Debugging through sourcemaps is not perfect, especially arrow functions. The generated sourcemaps look perfect; Chrome might be the problem. It might help to blacklist some babel transforms during development.

IntelliJ has a hard-coded 10 MB file size limit on sourcemaps it will load. My app.js.map is 3.5 MB and I'm sure there are apps out there 3x the size of mine.

@Turbo87
Copy link
Owner

Turbo87 commented Dec 10, 2016

@denofevil can you comment on that file size limit? this might become a problem for larger apps.

@denofevil
Copy link

@Turbo87 source maps loading does not use usual file limit

@dwickern
Copy link
Collaborator Author

This is the specific error I get loading my 15 MB vendor.js.map.

2016-12-09 13:11:46,318 [ 393215]  ERROR - jetbrains.rpc.CommandProcessor - File too big java.io.InputStreamReader@481e1976 
com.intellij.openapi.util.io.FileTooBigException: File too big java.io.InputStreamReader@481e1976
	at com.intellij.openapi.util.io.FileUtil.adaptiveLoadText(FileUtil.java:285)
	at com.jetbrains.javascript.debugger.sourcemap.SourceMapLoaderKt$loadRemoteSourceMapData$1.handleResponse(SourceMapLoader.kt:189)
	at com.jetbrains.javascript.debugger.sourcemap.SourceMapLoaderKt$loadRemoteSourceMapData$1.handleResponse(SourceMapLoader.kt)
	at org.apache.http.client.fluent.Response.handleResponse(Response.java:90)
	at com.jetbrains.javascript.debugger.sourcemap.SourceMapLoaderKt.loadRemoteSourceMapData(SourceMapLoader.kt:172)
	at com.jetbrains.javascript.debugger.sourcemap.SourceMapLoaderKt.findSourceMap(SourceMapLoader.kt:153)
	at com.jetbrains.javascript.debugger.JavaScriptDebugProcess.findSourceMapBeforeAdd(JavaScriptDebugProcess.kt:728)
	at com.jetbrains.javascript.debugger.JavaScriptDebugProcess$MyDebugEventListener.scriptAdded(JavaScriptDebugProcess.kt:551)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at com.intellij.util.EventDispatcher.dispatch(EventDispatcher.java:99)
	at com.intellij.util.EventDispatcher.access$200(EventDispatcher.java:35)
	at com.intellij.util.EventDispatcher$2.invoke(EventDispatcher.java:79)
	at com.sun.proxy.$Proxy107.scriptAdded(Unknown Source)
	at org.jetbrains.wip.WipScriptManager.scriptParsed(WipScriptManager.kt:96)
	at org.jetbrains.wip.WipVm$3.invoke(WipVm.kt:52)
	at org.jetbrains.wip.WipVm$3.invoke(WipVm.kt:16)
	at org.jetbrains.jsonProtocol.EventMap.handleEvent(EventMap.kt:49)
	at org.jetbrains.wip.WipCommandProcessor.eventEmitted(WipCommandProcessor.kt:33)
	at com.intellij.chromeConnector.extension.DebuggerService$invoke$2.invoke(DebuggerService.kt:118)
	at com.intellij.chromeConnector.extension.DebuggerService$invoke$2.invoke(DebuggerService.kt:67)
	at com.intellij.chromeConnector.extension.JbWipVm$queueProcessor$1.consume(JbWipVm.kt:16)
	at com.intellij.chromeConnector.extension.JbWipVm$queueProcessor$1.consume(JbWipVm.kt:13)
	at com.intellij.util.concurrency.QueueProcessor.lambda$null$0(QueueProcessor.java:105)
	at com.intellij.util.concurrency.QueueProcessor.runSafely(QueueProcessor.java:222)
	at com.intellij.util.concurrency.QueueProcessor.lambda$wrappingProcessor$1(QueueProcessor.java:105)
	at com.intellij.util.concurrency.QueueProcessor.lambda$null$2(QueueProcessor.java:202)
	at com.intellij.util.concurrency.QueueProcessor.runSafely(QueueProcessor.java:222)
	at com.intellij.util.concurrency.QueueProcessor.lambda$startProcessing$3(QueueProcessor.java:202)
	at com.intellij.openapi.application.impl.ApplicationImpl$2.run(ApplicationImpl.java:308)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:745)

@denofevil
Copy link

denofevil commented Dec 13, 2016

@dwickern what server is serving the files and what response headers does it provide for source maps? From the source code looks like if there's a contentLength header another branch is used that does not imply any limits on file size

@Turbo87
Copy link
Owner

Turbo87 commented Dec 13, 2016

@denofevil it is served by an express server using broccoli-middleware and at least in theory it should be served including the Content-Length header (see https://github.com/ember-cli/broccoli-middleware/blob/v0.18.1/lib/middleware.js#L100)

I'll try and verify this via wireshark to make sure.

@Turbo87
Copy link
Owner

Turbo87 commented Dec 13, 2016

here is what wireshark shows me:

    HTTP/1.1 200 OK\r\n
    X-Powered-By: Express\r\n
    Last-Modified: Tue, 13 Dec 2016 09:17:35 GMT\r\n
    Cache-Control: private, max-age=0, must-revalidate\r\n
    Content-Type: application/json\r\n
    Vary: Accept-Encoding\r\n
    Content-Encoding: gzip\r\n
    Date: Tue, 13 Dec 2016 09:19:12 GMT\r\n
    Connection: keep-alive\r\n
    Transfer-Encoding: chunked\r\n
    \r\n
    [Request in frame: 242]
    HTTP chunked response
    Content-encoded entity body (gzip): 858728 bytes -> 4346699 bytes

so it seems that the sourcemap is indeed not served with a Content-Length header because it's processed through a GZip compression and it looks like that is removing the header.

Note that this example is not using a 10MB+ sourcemap, but is just the sourcemap produced by an almost fresh ember new.

@denofevil
Copy link

@Turbo87 we're going to fix that on our side, but it's probably worth checking whether Content-Length can be produced on the server

@Turbo87
Copy link
Owner

Turbo87 commented Dec 13, 2016

@denofevil I'll look into that. Thanks for the feedback!

@Turbo87
Copy link
Owner

Turbo87 commented Dec 13, 2016

it appears that this is not supported by the compression middleware (see expressjs/compression#52) and maybe for a good reason after reading that thread... 🤔

@denofevil
Copy link

denofevil commented Dec 13, 2016

@Turbo87 yeah, sounds reasonable. Anyway, fix just landed on master, so you can expect it in 2017.1 EAP which is going to start pretty soon

@dwickern
Copy link
Collaborator Author

This LGTM, all that's left are some docs describing how to use this

@dwickern dwickern changed the title [WIP] integrated debugging integrated debugging Sep 5, 2017
@vi34
Copy link

vi34 commented Aug 9, 2018

@dwickern, @Turbo87 Why this PR isn't merged? Are there any troubles with IntelliJ?

@Turbo87
Copy link
Owner

Turbo87 commented Aug 9, 2018

the sourcemaps that Ember CLI produces by default are only mapping the ES-latest transforms, but not the AMD modules transform. @dwickern worked on making that configurable, but if I remember correctly there were some unresolved issues.

@vi34
Copy link

vi34 commented Aug 28, 2018

@dwickern Maybe you can merge this PR now, and resolve sourcemap issues as a separate issue?
Seems that this PR can add debugging ability at least in some cases.

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

Successfully merging this pull request may close these issues.

4 participants