Feature: Debugger callback on pause/resume, other events

[fatcerberus]

This would allow the target to suspend background operations such as audio playback on debugger pause (even if that pause is triggered by, e.g., an uncaught error) and restarting them on resume. Currently to do this the target must intercept its own outgoing debug messages and parse the status messages manually.

[svaarala]

Isn't this already available in the Status notifys?

[fatcerberus]

Yes, but as mentioned above the target has to parse its own messages, which is awkward if debugging is not normally done in-target.

The other option would be for the client to observe the status notifys and send an AppRequest to trigger audio pause etc. but it would be nice if the target could respond to the pause immediately.

[svaarala]

Ah, I see what you mean. The title wording was a bit misleading because "Notify the target" sounded like you meant adding a debugger notify. But this would be a native callback like the current detached callback.

Just looking at the incremental change needed, this feature would be very simple to add with a breaking change to duk_debugger_attach().

But as with many other things, I'm just a bit concerned about the potential for feature creep: what other things should the target be kept aware of re: debug state? Stepping? Uncaught errors? Whenever one adds something to the API, there's potential for adding a bunch of similar things until you run out of "similar things". The current detached callback is only there because the design won't work otherwise. So the current "policy" is for the set of callbacks to minimal, strictly for operating the debugger mechanics.

Adding callbacks is especially awkward because C APIs are not very easily versionable. Each callback either binary breaks the duk_debugger_attach() call, creates new variants of it that then need deprecating etc, or additional API calls to set callbacks after attach are needed. All rather heavy maintenance-wise. So something generic would be preferable.

One rather generic approach here would be to allow the target to essentially spy on the parsed debug messages somehow. Currently they don't have an internal representation however so this is not an easy option. It would be an option if debug messages were, for example, first parsed into the value stack and only then interpreted. The target could then spy on the debug request on the value stack generically without adding an ever increasing number of callbacks.

[fatcerberus]

I see what you mean, duk_debugger_attach() is already a massive callsite because of all the callbacks that have to be passed in. Somewhat unrelated to this issue, it maybe would make sense to have the callbacks passed in as a struct instead, similar to duk_memory_functions.

I agree that a more generic solution would be preferred over incrementally adding callbacks.

[svaarala]

duk_memory_functions are not passed as an in-argument - but they could be handled that way too.

A struct by itself wouldn't solve the versioning issue: there's no portable way to initialize unknown pointers to NULL (memzero is not always correct).

But if there was also an initialization macro, it might be workable:

duk_debugger_callbacks my_cb;

DUK_INIT_DEBUGGER_CALLBACKS(&my_cb);
my_cb.detached = something;
duk_debugger_attach(&my_cb);

[fatcerberus]

Maybe there could be a generic callback that Duktape would call into, with similar semantics to the AppRequest callback, whenever it parsed a full message. The value stack would then contain all the dvalues in the message (the marker such as REQ or NFY would have to provided as an argument I think)

[svaarala]

That's a possible approach, some practical issues with that:

• The application may need to spy on both the request and the response (maybe the response contains something that is interesting) for actual method requests. So also the response would need to be built onto the value stack first.
• The debugger code doesn't generally parse via the value stack now (and that's intentional to minimize memory churn). This would be a relatively big change.
• There may well be dvalues that don't have a corresponding value stack type. For example "none" cannot be represented on the value stack but it's useful in the debug protocol.
• If CBOR is adopted, then requests or at least responses may be structured. That wouldn't work well with the value stack approach unless the structured data was built into proper objects. That would be quite bad because the intent in the debugger code is to stream out values (with CBOR arbitrarily deep structured values could be streamed out with constant memory overhead).

So overall this seems a bit unworkable to me. At least the trade-off of being able to spy on the messages vs. the costs seems out of proportion.

One rather simple approach would be to just provide an extra that acts as a shim between your transport and Duktape, and would let you spy on the messages. It will have the memory churn issues mentioned above, but at least it wouldn't be part of the core solution. The shim would provide the callbacks required by Duktape, and call forward to an identical set of callbacks and handle the parsing / spying internally.

[fatcerberus]

Yeah, another practical issue is that Duktape doesn't generally "parse" outgoing messages as such, it just sends out the values as it goes.

[fatcerberus]

Duktape does have that extra that lets you parse messages on the target, right? Maybe that could be adapted for this purpose.

[svaarala]

That should work relatively easily I think.

But to be clear, that's really just "a solution" to the original problem. Not a very elegant one, I'm afraid, if one just considers the application writer and doesn't take into account the versioning and maintenance issues for Duktape development.

Hopefully a good idea for doing this in a versioning friendly way (like the application custom messages which I really like) comes along at some point :)

[fatcerberus]

In general it would be nice to have callbacks for various things. For example, minisphere implements source maps on-target for filenames: https://github.com/fatcerberus/minisphere/blob/v4.3.8/src/engine/debugger.c#L186-L209

That's easy to do by simply by compiling the scripts using their source tree names. There's currently no way to translate line numbers on the target side, though. Without the ability to intercept messages, I would have to download the source map from the target and parse it on the client side. (Thankfully that isn't as difficult now because of AppRequest. :)

But maybe more callbacks aren't the answer here - in general it's probably more robust to just translate the messages directly, which is currently possible.

[svaarala]

It's easy to add a bunch of callbacks, but much harder to add callbacks that survive versioning and don't grow without bounds ;)

[fatcerberus]

Definitely agree there.

Luckily I'll be able to sidestep the source map issue for minisphere 4.4.0 because Babel provides a retainLines option that works pretty well. The transpiled line numbers match the source line numbers 1:1 (assuming no minification), so I only need to translate the filenames.

[svaarala]

That's a very nice option and much simpler than source maps or try-catch-tweak-error magic.