Skip to content

remove com.sun.jdi.VMDisconnectedException from reasonable places.#104

Merged
andxu merged 22 commits intomasterfrom
andy_log_attach
Dec 5, 2017
Merged

remove com.sun.jdi.VMDisconnectedException from reasonable places.#104
andxu merged 22 commits intomasterfrom
andy_log_attach

Conversation

@andxu
Copy link
Copy Markdown
Contributor

@andxu andxu commented Nov 14, 2017

#59

yaohaizh
yaohaizh previously approved these changes Nov 14, 2017
org.eclipse.jdi.internal.connect.PacketSendManagersendPacket at PacketSendManager.java Line 90
        org.eclipse.jdi.internal.MirrorImplrequestVM at MirrorImpl.java Line 187
        org.eclipse.jdi.internal.MirrorImplrequestVM at MirrorImpl.java Line 268
        org.eclipse.jdi.internal.VirtualMachineImplgetIDSizes at VirtualMachineImpl.java Line 984
        org.eclipse.jdi.internal.VirtualMachineImplpacketSendManager at VirtualMachineImpl.java Line 343
        org.eclipse.jdi.internal.MirrorImplrequestVM at MirrorImpl.java Line 187
        org.eclipse.jdi.internal.MirrorImplrequestVM at MirrorImpl.java Line 268
        org.eclipse.jdi.internal.VirtualMachineImplgetVersionInfo at VirtualMachineImpl.java Line 1010
        org.eclipse.jdi.internal.VirtualMachineImplversion at VirtualMachineImpl.java Line 939
        com.microsoft.java.debug.core.adapter.handler.AttachRequestHandlerhandle at AttachRequestHandler.java Line 65
        com.microsoft.java.debug.core.adapter.DebugAdapterdispatchRequest at DebugAdapter.java Line 80
        com.microsoft.java.debug.core.adapter.ProtocolServerdispatchRequest at ProtocolServer.java Line 66
        com.microsoft.java.debug.core.protocol.AbstractProtocolServerprocessData at AbstractProtocolServer.java Line 166
        com.microsoft.java.debug.core.protocol.AbstractProtocolServerstart at AbstractProtocolServer.java Line 80
        com.microsoft.java.debug.core.adapter.ProtocolServerstart at ProtocolServer.java Line 53
        com.microsoft.java.debug.plugin.internal.JavaDebugServer$2run at JavaDebugServer.java Line 136
        java.util.concurrent.Executors$RunnableAdaptercall at Executors.java Line 514
        java.util.concurrent.FutureTaskrun at FutureTask.java Line 264
        java.util.concurrent.ThreadPoolExecutorrunWorker at ThreadPoolExecutor.java Line 1167
        java.util.concurrent.ThreadPoolExecutor$Workerrun at ThreadPoolExecutor.java Line 641
        java.lang.Threadrun at Thread.java Line 844
@andxu andxu changed the title add logs for attaching errors. remove com.sun.jdi.VMDisconnectedException from reasonable places. Nov 14, 2017
yaohaizh
yaohaizh previously approved these changes Nov 17, 2017
context.sendEvent(Events.OutputEvent.createConsoleOutput(warnMessage));
}
}
} catch (VMDisconnectedException e) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This exception seems like a common one for all the handlers. Can we handle them at the framework level?

…happening, since it is normal to have received terminated message during process vscode requests, return an empty response since the debug session is terminated.
try {
vm.eventRequestManager().deleteEventRequests(requests());
} catch (VMDisconnectedException ex) {
// ignore since removing breakpoints is meanness when JVM is terminated.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typos. meaningless

# Conflicts:
#	com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/DebugAdapter.java
#	com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/handler/ThreadsRequestHandler.java
# Conflicts:
#	com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/DebugAdapter.java
#	com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/handler/ThreadsRequestHandler.java
testforstephen
testforstephen previously approved these changes Nov 22, 2017
@@ -68,16 +70,32 @@ protected void dispatchRequest(Messages.Request request) {
this.debugAdapter.dispatchRequest(request).whenComplete((response, ex) -> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use thenAccept.

"The request dispatcher should not return null response."));
}
}
}).whenComplete((r, e) -> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use exceptionally().

ErrorCode.UNKNOWN_FAILURE,
"The request dispatcher should not return null response."));
response = new Messages.Response(request.seq, request.command);
if (ex != null) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put the exception handling logic into the exceptionally() handler.

@@ -68,16 +70,32 @@ protected void dispatchRequest(Messages.Request request) {
this.debugAdapter.dispatchRequest(request).whenComplete((response, ex) -> {
if (response != null) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't need to check response for null value when using thenAccept.

@akaroml akaroml added this to the 0.4.0 milestone Nov 29, 2017
andxu added 2 commits December 1, 2017 02:30
# Conflicts:
#	com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/ProtocolServer.java
#	com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/handler/ThreadsRequestHandler.java
# Conflicts:
#	com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/ProtocolServer.java
#	com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/handler/ThreadsRequestHandler.java
@andxu andxu merged commit 55e29be into master Dec 5, 2017
@andxu andxu deleted the andy_log_attach branch March 15, 2018 01:45
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

Successfully merging this pull request may close these issues.

4 participants