Skip to content

Commit 180cfd9

Browse files
committed
dap: pass exit code through exited event
Pass the exit code through the exited event back to the client and ensure that the printed text is printed completely. Previously, the exited event just had a big todo and the printer would sometimes fail to send messages to the connected client. This moves the printer wait to before the debug adapter is closed to ensure that all messages get sent through the connection to the editor. While there, I also plumbed in the exit code to exited. It's not necessarily the real exit code but it will produce a zero on build success and a non-zero code on build failure so that should be good enough. Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
1 parent ba04f8f commit 180cfd9

6 files changed

Lines changed: 106 additions & 20 deletions

File tree

commands/build.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -398,10 +398,6 @@ func runBuild(ctx context.Context, dockerCli command.Cli, debugOpts debuggerOpti
398398
done := timeBuildCommand(mp, attributes)
399399
resp, inputs, retErr := runBuildWithOptions(ctx, dockerCli, opts, dbg, printer)
400400

401-
if err := printer.Wait(); retErr == nil {
402-
retErr = err
403-
}
404-
405401
done(retErr)
406402
if retErr != nil {
407403
return retErr
@@ -462,12 +458,21 @@ func runBuildWithOptions(ctx context.Context, dockerCli command.Cli, opts *Build
462458
if err := dbg.Start(printer, opts); err != nil {
463459
return nil, nil, err
464460
}
465-
defer dbg.Stop()
461+
defer func() { dbg.Stop(retErr) }()
466462

467463
bh = dbg.Handler()
468464
dockerCli.SetIn(nil)
469465
}
470466

467+
// Ensure messages sent to the printer are flushed before the debugger completes.
468+
// This prevents late messages from not being sent because the connection was
469+
// terminated before completion of the debugger.
470+
defer func() {
471+
if err := printer.Wait(); retErr == nil {
472+
retErr = err
473+
}
474+
}()
475+
471476
in := dockerCli.In()
472477
for {
473478
resp, inputs, err := RunBuild(ctx, dockerCli, opts, in, printer, &bh)

commands/dap.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,9 @@ func (d *adapterProtocolDebugger) Start(printer *progress.Printer, opts *BuildOp
9191
return nil
9292
}
9393

94-
func (d *adapterProtocolDebugger) Stop() error {
94+
func (d *adapterProtocolDebugger) Stop(retErr error) error {
9595
defer d.conn.Close()
96-
return d.Adapter.Stop()
96+
return d.Adapter.Stop(retErr)
9797
}
9898

9999
func dapAttachCmd() *cobra.Command {

commands/debug.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ type debuggerOptions interface {
4141
type debuggerInstance interface {
4242
Start(printer *progress.Printer, opts *BuildOptions) error
4343
Handler() build.Handler
44-
Stop() error
44+
Stop(retErr error) error
4545
Out() io.Writer
4646
}
4747

@@ -98,7 +98,7 @@ func (d *monitorDebuggerInstance) Handler() build.Handler {
9898
return d.m.Handler()
9999
}
100100

101-
func (d *monitorDebuggerInstance) Stop() error {
101+
func (d *monitorDebuggerInstance) Stop(_ error) error {
102102
return d.m.Close()
103103
}
104104

dap/adapter.go

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ func (d *Adapter[C]) Start(conn Conn) (C, error) {
8383
return resp.Config, resp.Error
8484
}
8585

86-
func (d *Adapter[C]) Stop() error {
86+
func (d *Adapter[C]) Stop(retErr error) error {
8787
if d.eg == nil {
8888
return nil
8989
}
@@ -94,15 +94,27 @@ func (d *Adapter[C]) Stop() error {
9494
Event: "terminated",
9595
},
9696
}
97-
// TODO: detect exit code from threads
98-
// c.C() <- &dap.ExitedEvent{
99-
// Event: dap.Event{
100-
// Event: "exited",
101-
// },
102-
// Body: dap.ExitedEventBody{
103-
// ExitCode: exitCode,
104-
// },
105-
// }
97+
98+
// Send an exit code based on the returned error.
99+
// Any error results in sending an exit code of 1 while
100+
// no error sends zero for success.
101+
//
102+
// The exited event is sent after the terminated event.
103+
// See the specification overview diagram on the bottom of the page
104+
// for a detailed flowchart.
105+
// https://microsoft.github.io/debug-adapter-protocol/overview
106+
exitCode := 0
107+
if retErr != nil {
108+
exitCode = 1
109+
}
110+
c.C() <- &dap.ExitedEvent{
111+
Event: dap.Event{
112+
Event: "exited",
113+
},
114+
Body: dap.ExitedEventBody{
115+
ExitCode: exitCode,
116+
},
117+
}
106118
})
107119
d.srv.Stop()
108120

dap/adapter_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ func NewTestAdapter[C LaunchConfig](t *testing.T) (*Adapter[C], Conn, *daptest.C
253253
client := daptest.NewClient(clientConn)
254254
t.Cleanup(func() { client.Close() })
255255

256-
t.Cleanup(func() { adapter.Stop() })
256+
t.Cleanup(func() { adapter.Stop(nil) })
257257
return adapter, srvConn, client
258258
}
259259

tests/dap_build.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"os"
66
"os/exec"
77
"path"
8+
"path/filepath"
89
"runtime"
910
"slices"
1011
"strings"
@@ -87,6 +88,7 @@ var dapBuildTests = []func(t *testing.T, sb integration.Sandbox){
8788
testDapBuildStepOut,
8889
testDapBuildVariables,
8990
testDapBuildDeferredEval,
91+
testDapBuildExitedEvent,
9092
}
9193

9294
func testDapBuild(t *testing.T, sb integration.Sandbox) {
@@ -914,6 +916,73 @@ func testDapBuildDeferredEval(t *testing.T, sb integration.Sandbox) {
914916
require.ErrorAs(t, done(true), &exitErr)
915917
}
916918

919+
func testDapBuildExitedEvent(t *testing.T, sb integration.Sandbox) {
920+
t.Run("success", func(t *testing.T) {
921+
dir := createTestProject(t)
922+
client, done, err := dapBuildCmd(t, sb)
923+
require.NoError(t, err)
924+
925+
ch := make(chan *dap.ExitedEvent, 1)
926+
client.RegisterEvent("exited", func(em dap.EventMessage) {
927+
ch <- em.(*dap.ExitedEvent)
928+
close(ch)
929+
})
930+
931+
// Project should just build normally.
932+
doLaunch(t, client, commands.LaunchConfig{
933+
Dockerfile: path.Join(dir, "Dockerfile"),
934+
ContextPath: dir,
935+
})
936+
937+
select {
938+
case exited := <-ch:
939+
require.Equal(t, 0, exited.Body.ExitCode)
940+
case <-time.After(5 * time.Second):
941+
require.Fail(t, "timeout reached")
942+
}
943+
944+
require.NoError(t, done(true))
945+
})
946+
947+
t.Run("failure", func(t *testing.T) {
948+
dir := createTestProject(t)
949+
client, done, err := dapBuildCmd(t, sb)
950+
require.NoError(t, err)
951+
952+
ch := make(chan *dap.ExitedEvent, 1)
953+
client.RegisterEvent("exited", func(em dap.EventMessage) {
954+
ch <- em.(*dap.ExitedEvent)
955+
close(ch)
956+
})
957+
958+
// Delete foo from the test project so this will fail.
959+
err = os.Remove(filepath.Join(dir, "foo"))
960+
require.NoError(t, err)
961+
962+
interruptCh := pollInterruptEvents(client)
963+
doLaunch(t, client, commands.LaunchConfig{
964+
Dockerfile: path.Join(dir, "Dockerfile"),
965+
ContextPath: dir,
966+
})
967+
968+
// We will hit an interrupt because of the failure.
969+
stopped := waitForInterrupt[*dap.StoppedEvent](t, interruptCh)
970+
require.Equal(t, "exception", stopped.Body.Reason)
971+
972+
// Continue execution which should trigger the exited event.
973+
doNext(t, client, stopped.Body.ThreadId)
974+
select {
975+
case exited := <-ch:
976+
require.NotEqual(t, 0, exited.Body.ExitCode)
977+
case <-time.After(time.Second):
978+
require.Fail(t, "timeout reached")
979+
}
980+
981+
var exitErr *exec.ExitError
982+
require.ErrorAs(t, done(false), &exitErr)
983+
})
984+
}
985+
917986
func doLaunch(t *testing.T, client *daptest.Client, config commands.LaunchConfig, bps ...dap.SourceBreakpoint) {
918987
t.Helper()
919988

0 commit comments

Comments
 (0)