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

Do not send 'STOPPED' event before sending the reponse for requests from VSCode #37

Closed
isidorn opened this issue May 25, 2016 · 1 comment
Labels

Comments

@isidorn
Copy link

isidorn commented May 25, 2016

After investigating microsoft/vscode#6757 it seems that the PHP adapter is sending a STOPPED event before the reponse for the step command.

Previously we had a workaround that would prevent this issue, but we have removed this workaround due to other issues microsoft/vscode#5966

The order of respone / events now needs to be respected, and the PHP adapter should only send the 'STOPPED' event after sending back the response for commands to VSCode.

fyi @weinand

@felixfbecker
Copy link
Contributor

felixfbecker commented May 25, 2016

I wasn't aware of this limitation, I thought an event can be send at any time. See my implementation of configurationDoneRequest:

    /** Executed after all breakpoints have been set by VS Code */
    protected async configurationDoneRequest(response: VSCodeDebugProtocol.ConfigurationDoneResponse, args: VSCodeDebugProtocol.ConfigurationDoneArguments) {
        try {
            for (const connection of this._waitingConnections) {
                // either tell VS Code we stopped on entry or run the script
                if (this._args.stopOnEntry) {
                    // do one step to the first statement
                    const response = await connection.sendStepIntoCommand();
                    this._checkStatus(response, true);
                } else {
                    const response = await connection.sendRunCommand();
                    this._checkStatus(response);
                }
                this._waitingConnections.delete(connection);
            }
        } catch (error) {
            this.sendErrorResponse(response, <Error>error);
            return;
        }
        this.sendResponse(response);
    }

where _checkStatus is:

   /**
     * Checks the status of a StatusResponse and notifies VS Code accordingly
     * @param {xdebug.StatusResponse} response
     * @param {boolean} [entry=false] if true and XDebug stopped because of a step command, the stoppedEvent reason will be 'entry', not 'step'
     */
    private async _checkStatus(response: xdebug.StatusResponse, entry: boolean = false) {
        const connection = response.connection;
        this._statuses.set(connection, response);
        if (response.status === 'stopping') {
            const response = await connection.sendStopCommand();
            this._checkStatus(response);
        } else if (response.status === 'stopped') {
            this._connections.delete(connection.id);
            this.sendEvent(new vscode.ThreadEvent('exited', connection.id));
            connection.close();
        } else if (response.status === 'break') {
            // StoppedEvent reason can be 'step', 'breakpoint', 'exception' or 'pause'
            let stoppedEventReason: 'step' | 'breakpoint' | 'exception' | 'pause' | 'entry';
            let exceptionText: string;
            if (response.exception) {
                stoppedEventReason = 'exception';
                exceptionText = response.exception.name + ': ' + response.exception.message; // this seems to be ignored currently by VS Code
            } else if (entry) {
                stoppedEventReason = 'entry';
            } else if (response.command.indexOf('step') === 0) {
                stoppedEventReason = 'step';
            } else {
                stoppedEventReason = 'breakpoint';
            }
            this.sendEvent(new vscode.StoppedEvent(stoppedEventReason, connection.id, exceptionText));
        }
    }

As you can see, I wrap my method in try/catch and then decide if I should call sendResponse or sendErrorResponse depending on wether an error occured or not. _checkStatus may in the meantime already send an event. It would be quite an effort for me to refactor everything so that events are deferred after responses.

@weinand I think this would be much better suited in the implementation of vscode-debugadapter. E.g. at the beginning dispatchRequest, set a flag awaitingResponse. Inside sendEvent, check if awaitingResponse is true, and if yes, push the event on a queue. When sendResponse is called, send the response, then set the flag to false and call sendEvent for every element in the queue, then clear it.

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

No branches or pull requests

2 participants