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

Process stops when log message and condition are both set #1146

Closed
msmans opened this issue Dec 8, 2022 · 4 comments
Closed

Process stops when log message and condition are both set #1146

msmans opened this issue Dec 8, 2022 · 4 comments

Comments

@msmans
Copy link

msmans commented Dec 8, 2022

Environment data

  • debugpy version: 1.6.4
  • OS and version: ArchLinux 6.1.0-rc8-1-mainline
  • Python version (& distribution if applicable, e.g. Anaconda): Python 3.10.8
  • Using VS Code or Visual Studio: VS Code 1.73.1 6261075646f055b99068d3688932416f2346dd3b x64

Actual behavior

When setting a log point with a condition, the log message is printed when the line is hit, but the debugger also stops the process.

Expected behavior

If log message is set, process must not be stopped (per Debugger Adapter Protocol spec):

If this attribute (ie logMessage) exists and is non-empty, the debug adapter must not 'break' (stop) but log the message instead.

The spec does not make an exception to this when conditon or hit count is specified.

Steps to reproduce:

  1. Open any python project.
  2. Right click the red dot on any line and select to Log Message.
  3. Enter an expression to log.
  4. Add condition as well.
  5. Start debugger.

Some DAP protocol logs:

{"command":"setBreakpoints","arguments":{"source":{"path":"/home/bob/file.py","sourceReference":0},"lines":[120],"breakpoints":[{"line":120,"condition":"{request.GET.get('id') == 1}","logMessage":"ID is set!"}],"sourceModified":false},"type":"request","seq":131}

{"seq": 293, "type": "response", "request_seq": 131, "success": true, "command": "setBreakpoints", "body": {"breakpoints": [{"verified": true, "id": 20, "source": {"path": "/home/bob/file.py", "sourceReference": 0}, "line": 120}]}}

{"seq": 296, "type": "event", "event": "output", "body": {"output": "ID is set!\n", "category": "stdout", "source": {}}}

{"seq": 297, "type": "event", "event": "stopped", "body": {"reason": "breakpoint", "threadId": 211, "preserveFocusHint": false, "allThreadsStopped": true}}

Notice the last stopped event which should not be there.

@fabioz
Copy link
Collaborator

fabioz commented Dec 8, 2022

I'm not sure.

This actually seems an oversight in the spec for me (in that it doesn't say how interactions among condition, hitCondition, logMessage work).

The current behavior IMHO is better... I created microsoft/debug-adapter-protocol#363 to ask for more details.

@msmans
Copy link
Author

msmans commented Dec 8, 2022

Thanks.

The current behavior IMHO is better

Interesting. I think the opposite. In fact, it was confusing to me that specifying a log message would continue to break, even before I read the specs. If a breakpoint stops the program, I could simply inspect the memory and no longer need to log. As I understand logging points, they are an alternative to stopping the application (which the adapter spec seems to agree with) for situations where stop and start is too time consuming.

I actually think having it only break if logMessage is not specified is a more flexible option. In other words, it's superset of the current behavior of debugpy. A hot code path can produce too much noise when logging. Being able to only log under certain conditions is useful. A real world example is my need to log certain requests among 10K+ which satisfy certain headers.

@fabioz
Copy link
Collaborator

fabioz commented Dec 8, 2022

@msmans I think it'd be nice if you can add your thoughts to microsoft/debug-adapter-protocol#363 as debugpy will end up implementing what's decided there...

fabioz added a commit to fabioz/debugpy that referenced this issue Dec 16, 2022
fabioz added a commit to fabioz/debugpy that referenced this issue Dec 16, 2022
fabioz added a commit to fabioz/debugpy that referenced this issue Dec 16, 2022
fabioz added a commit to fabioz/debugpy that referenced this issue Dec 16, 2022
fabioz added a commit to fabioz/debugpy that referenced this issue Dec 16, 2022
@fabioz
Copy link
Collaborator

fabioz commented Dec 16, 2022

Note: I've provided a PR to change it so that the debugger will never stop at logpoints: #1157

@fabioz fabioz closed this as completed in c419d5a Dec 17, 2022
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

2 participants