How do you implement pipeline steps that

[opal marten]

Its uncanny how this just came up somewhere else today.

@last talon I think the approach you came up with for the goreleaser PR makes sense to me.

@ember glacier what do you think?

https://github.com/kpenfound/goreleaser/blob/daggerize/dagger/test.go#L18

[ember glacier]

What should I look at exactly?

I mean, I don't really see the solution. If the go test command fails, CoverageReport isn't going to return anything as far as I can tell.

Here is why:

dagger core container from --address alpine with-new-file --path '/foo' --contents 'bar' with-exec --args 'baz' file --path '/foo'

[opal marten]

I'd like Kyle to speak to this when he gets a chance but the main thing here is that it returns a Container and allows you to dynamically access the bits you need in case of failure.

[ember glacier]

I may be missing something, but I don't understand how that's possible after a WithExec fails with a non-zero exit code.

[jolly nova]

@ember glacier I'm on a call but I've implemented this pattern quite a lot, will share notes shortly

[ember glacier]

Thanks!

[jolly nova]

@ember glacier so, a few things:

1. The core API does not handle this cleanly. It has a pet pieve of mine for a while, but anyway that's the situation at the moment.
2. I do this in domain-specific modules. Some tools actually have a flag for this, which avoids hacks. Example with golangci-lint: https://github.com/dagger/dagger/blob/main/modules/golangci/lint.go#L115
3. When I do need to resort to a hack, I run a shell script with || true. In practice it's fine. Doing it at the level of a domain-specific module makes it particularly benign. Example for a markdown linter: https://github.com/dagger/dagger/blob/main/modules/markdown/main.go#L58
4. I would like to define a standard Checker interface or equivalent, and start using that for easier aggregation of many checks with less boilerplate. Ultimately this could become a dagger check convenience. Want to collaborate on that? 🙂 I think a checker could be a test suite, lint run, code formatter, or anything else that outputs "green / red" + a report

As for markdown reports and reducing noise. May I suggest also using my Github Actions config which generates a markdown summary out of the box 🙂 Example: https://github.com/cubzh/cubzh/actions/runs/10409324152#summary-28828642977

[ember glacier]

Thanks @jolly nova !

What I'm missing from the above is a way to communicate that the pipeline in fact failed (for example: there were lint violations). If I understand it correctly, the above approach leads to a green CI even when something breaks.

Want to collaborate on that?
I'd love to!

As for markdown reports and reducing noise. May I suggest also using my Github Actions config which generates a markdown summary out of the box

I'll take a look

    // +defaultPath="/"
    // +ignore=["*", "!core", "!deps/libz"]

Did I miss something?

[jolly nova]

That's just me manifesting context dir

(these comments won't work until context dir ships)

[ember glacier]

Ah, okay 🙂

[jolly nova]

What I'm missing from the above is a way to communicate that the pipeline in fact failed (for example: there were lint violations). If I understand it correctly, the above approach leads to a green CI even when something breaks.

IMO with this pattern, you decouple the work of producing a test report, from the work of querying the report and returning an error (or not).

So initially, to preserve the same interface to CI, you would call a higher-level dagger function that queries the report and returns an error to CI like before.

But later, you might decide to update the PR status directly from the Dagger function, instead of letting the CI runner do it. For example Circle CI started pushing this pattern where you upload test reports to their test visualization service. Of course they do it for lock-in reason, but we can copy this pattern and apply it to portability 🙂

Then of course there are use cases beyond CI, for example generating a test matrix grid, where the concept of "failed" or "passed" is irrelevant, you just generate a report for the user to browse separately.

TLDR: decouple decouple decouple

[jolly nova]

For example, here's how to query the golangci linter for details:

dagger call -m github.com/dagger/dagger/modules/golangci \
 lint --source https://github.com/dagger/dagger \
 issues \
 summary

You can export it as structured data for further processing outside dagger:

dagger call --json -m github.com/dagger/dagger/modules/golangci \
 lint --source https://github.com/dagger/dagger \
 issues \
 summary \
| jq .

Example output:

[
  {
    "summary": "[gosec] cmd/dagger/flags.go:495: G204: Subprocess launched with a potential tainted input or cmd arguments"
  },
  {
    "summary": "[exportloopref] cmd/dagger/cloud.go:98: exporting a pointer for the loop variable org"
  },
  {
    "summary": "[exportloopref] dagql/objects.go:132: exporting a pointer for the loop variable field"
  },
  {
    "summary": "[exportloopref] engine/buildkit/executor_spec.go:414: exporting a pointer for the loop variable mnt"
  }
]

As a convenience if you just want to decide whether to return an error, you can get the error count:

dagger call -m github.com/dagger/dagger/modules/golangci \
 lint --source https://github.com/dagger/dagger \
 error-count

[twin vigil]

@jolly nova @grave rivet asked the same thing here today https://discord.com/channels/707636530424053791/1275080892410499163

What I think the above pattern is missing is the ability to get both, the status report and making the dagger CLI to error at the same time in the same dagger call

So if you're doing something like this today: golangci-lint ./foo 2> report.json and your lint fails, you get both the report and correct status code in the same step. IIUC, this is not possible with a single dagger call. You'd have return some struct with both the report and the error code, and use the --json flag to parse it from some wrapper command

[ember glacier]

IMO with this pattern, you decouple the work of producing a test report, from the work of querying the report and returning an error (or not).

Yeah, I understand that part. But I also think it's pretty unconventional and a huge shift from how people built CI pipelines.

But later, you might decide to update the PR status directly from the Dagger function, instead of letting the CI runner do it.

I experimented with that. It isn't necessarily a bad approach, but then you need to start thinking about retries to ensure results are correctly reported.

That's what actually keeps me from going down this path right now: I just wanted to generate a frickin test report and now I have to rethink how results are reported back.

[jolly nova]

what do you want to do with the test report ultimately? maybe start from there and go backwards

eg. if you want to export it locally then have the CI script upload it somewhere - then move the upload phase into dagger. etc

[ember glacier]

I mostly want it in CI, so it’s easier to navigate to the code causing failure (eg. code violating lint rule).

Locally users can always execute parts of the pipeline to resuce noise

[jolly nova]

but how exactly do you feed it into ci for navigation?

[ember glacier]

GitHub Job summary. Lint report tells me where, I can create a link to the file.

Same for tests.

[jolly nova]

But isn't the runner's local filesystem ephemeral? Don't you need to upload the file to a persistent location? Sorry I'm not a gha expert. Any chance you could share a config snippet?

[ember glacier]

run: echo '### Hello world! :rocket:' >> $GITHUB_STEP_SUMMARY

I would generate the report and export it as a file to the runner filesystem

[jolly nova]

so you would print the text contents of the report file as part of the markdown report?

[ember glacier]

yep

[jolly nova]

like in a markdown quote block perhaps?

[ember glacier]

No, the exact same way your function generates the summary 😄

dagger call ci export --path report.md > report.md

cat report.md > $GITHUB_STEP_SUMMARY

[jolly nova]

ah I see.

Here's why I'm asking: https://github.com/cubzh/cubzh/actions/runs/10463134517#summary-28974643642

[ember glacier]

BUT, I'd still want the ci job to result in a failure

[jolly nova]

yeah I see

we do have a blind spot there

[ember glacier]

I want to use the test and lint reports generated inside my ci function. In fact I want to generate the report.md file from inside a function.

And ideall, I would want Dagger to signal which step caused the failure. Simply returning exit 1 after the function exits wouldn't do that.

[jolly nova]

maybe we could have a standard interface for CI integrations. If you return a type that implements that interface, CI integrations are expected to use it to get the information they need (eg. job status)

in fact maybe that interface is the Check interface I mentioned earlier in the thread

[ember glacier]

Well, I might be oversimplifying things, but in my mind, a similar construct as "continue on error" would be useful. In Dagger terms: WithExec().File() could still return the file if there is one. Dunno if that's feasible.

[jolly nova]

Yeah but that's not the real blocker (you can use the tricks I described before). It's cosmetic. The real blocker is that you can return a string, or file, back to the CLI caller

And no it's not feasible, at least not easily. I think it's tightly coupled to how the engine implements the GraphQL spec. You make a query, if it fails, you get an error.

[ember glacier]

Well, if pointing to the right step in the pipeline as the cause of failure is cosmetics, then I guess it is.

[jolly nova]

No what's cosmetic is how you do it. You can already achieve that by wrapping in a shell script, and if needed, getting the exit code out to decide to throw an error

[ember glacier]

I'm gonna give those workarounds a try, but it's gonna take some getting used to. It's quite...unusual.

[jolly nova]

My point was that there is a more pressing problem to solve than that WithExec. It's your problem of getting the report out to CI and also reporting the error. The workaround won't help with that. We need to find a solution to that together.

[ember glacier]

Yep

[twin vigil]

This. It's the same blocker I mentioned in my comment above

[jolly nova]

indeed 🙂

[wooden brook]

Thanks for the support and replies @dark yoke @twin vigil @jolly nova

Its very common in the PHP ecosystem to get the PHP coverage report, even if the suite fails or not, and we can't really ask people to migrate over to Dagger until this is possible.

Coverage reports are part of the Unit testing suite, which is typically the 2nd thing ran

1. Linting + Static Analysis
2. Unit
3. Functional/Integration
4. Acceptance/API/UI

In the PHP SDK we're able to run the static analysis, no problem, so it's just Unit related features that need to be here (prioritized hopefully?) so we can progress in the "Readiness of Dagger" that the PHP community (and I'm sure wider communities) will be evaluating.

[last talon]

I'm late to this thread but I think it's still valuable to talk about the workaround I've tried. It's similar to the workarounds noted above but not exactly the same I think. In the function that Lev linked, it turned out to not need the report if the tests failed, so I ended up removing the convenience functions at some point.

The way my workaround is a bit different is that it just used 2 functions to accomplish the report + error rather than the various ways above to try to get this in one function.

It looked something like

dagger call test ... report -o ./report.xml # <-- this gets the report no matter what the test status is

# then, call a different function on the test interface

dagger call test ... success # <-- this just does a sync on the test command, so the exit code will reflect the test results and avoids parsing json

[jolly nova]

@wide notch re: alwaysExec. Sometimes the test tool has a flag to not return an error on failed tests, that makes the shell wrapper unnecessary. So in my experience it's a case-by-case thing

[jolly nova]

See examples 👆

Also the alwaysExec won't solve the problem of the entrypoint function. The workaround we're discussing is very similar to your custom TestResult type idea. I've been calling it Check because it can also include linters and other green/red use cases beyond tests. But same general idea.

@wooden brook is correct that a custom type is specific to just you... Unless we make it an interface, then work together to define a convention. Then we can all implement the same interface, both on the callee side (dagger functions that return that type) and the caller side (CI integrations that know what to do with it).

[wide notch]

not sure any php test tools have this flag so it'd be something we'd need often enough to at least justify abstracting to a module

[wooden brook]

withExecAllowFail()

🙃

What do you think of my name suggestion ?

[wide notch]

we can come back to that 🙂

perhaps we need to be able to differentiate between withExec -> that is the equivilent of doing a dockerfile run and is not expected to fail and a runThisThing which is part of the validation of the build which may fail and you may want to handle that in your own way but still have the step as a whole marked as a failure

[jolly nova]

that's pretty good!

[dark yoke]

starting to think about technical implementations of doing this in the core api (no promises, just trying to scope right now)
generally, for php tooling is the list of "valid" exit codes for a tool known? e.g. is it something like 0 on success, 1 on fail? or is it more like 0 on success, entirely random exit code on fail?

my reasoning is - i could see a field that was like "ValidExitCodes" that's a list, and you could pass [0, 1] to it. i'm not sure about a generic "allow all exit codes here"

additionally, all exit codes < 0 should probably always fail regardless right? since those mean the process was terminated by a signal (like SIGKILL/SIGILL/SIGSEGV)

[wide notch]

I think it's reasonable to assume that the exit codes are knowable; a valid exit codes would be only a half fix as you'd still want to be able to get the exit code to know if the step completed or not

I think some sort of Check interface is going to be the way to go

This is one possible option: https://github.com/charjr/DaggerAlwaysExec/tree/main

though that relies on saving the execution details inside the container for retrevial - ideally something in core could store them externally for retrevial as part of a dagger object

[opal marten]

Yes, and this is how most (if not all) CI systems work today. They determine success or failure exclusively on any non-zero exit code

[wooden brook]

@opal marten @last talon @jolly nova

Hey 👋 I'm picking up this topic again.

Yesterday I watched the dagger community call, and the speaker (2nd person?) Was exeutung typescript tedt suite.. with failing tests ..

1. He was showing the code coverage report .. I'm wondering how he managed to get the coverage report outside of the running dagger container, if it failed?

2. Where are we at, in general, with a permanent fix for what we are discussing in this discord thread?

Thanks 👍

[last talon]

I'm not sure exactly which demo you're referring to, but it's most likely using a pattern like this: https://docs.dagger.io/cookbook#continue-using-a-container-after-command-execution-fails

[wooden brook]

Hey @last talon

Yep. I'm aware of this technique. We diacussiled it above, by trapping ?$
I'm wondering if we came up with an actual solution to it rather than this workaround thing.

Looks like we haven't yet.

I'm wondering what's the priority of this, on our dagger roadmap and if we have an ETA on a solution to this.

For example in Jenkins we can do

code = sh(cmd, return_code=true)

publishReport()

return code;

[wooden brook]

@last talon we need a user friendly dagger handler for this.

The majority of CI test runs will be false. And the final one(s) will be green.

So making this "just work" is really important to DX, and the adoption of dagger.

A custom object like @wide notch mentioned or similar can be fine for people using that instead of the withExec() because we know we are calling a test suite that can fail, and that's okay, whilst we do cleanups and report publishing.

What are your thoughts ?

[opal marten]

Yeah I think we either need a new primitive or update the existing with_exec to handle this somehow. This came up in conversation this morning with someone else I was talking to live.

[jolly nova]

Guys like I said before, with_exec is not actually the biggest blocker - it's an inconvenience sure, but there is also a bigger blocker, which is the dagger call behavior which will either return an error or a result to the CLI caller, but not both

and we don't have a fix in flight for either of these things

cc @silk pine @sterile swallow to put this on your radar

[wide notch]

some sort of result primative would be the way to go

[dark yoke]

made some very good progress on this today: https://github.com/dagger/dagger/pull/8466
❤️ hopefully that should resolve y'alls issues with this 😄

[silk pine]

will this mean that those nonzero exit code results become cached?

[dark yoke]

🤔 yes

i think that makes sense?

e.g. if you're doing a lint report, and it could return 0 or 1, then you want it to cache if the files haven't changed

is there a case you wouldn't want it to cache?

imo, i do think we should be able to mark individual steps as no-cache as a future extension, but in the meantime, i think if the exit code is in the specified list, we should treat the step as "succeeded", and cache it

(as an aside, i have no idea how to determine whether to cache a step after it's executed - that's gonna be some insane buildkit scheduling work i think)

[flint pendant]

I agree; as long you're explicitly specifying which exit codes are allowed.

[silk pine]

I'm just thinking of test flakes, but that might be a different use case where you wouldn't want to use this anyway. Makes sense for lints, since those don't "flake" ( )

[dark yoke]

mm, this is a good point

i do think that the "right" way to solve this would be by using a cache buster though - if you expect flakes, then you should never cache the results, even successes

just caching the successes and not the failures gives you a misguided view of how stable your test suite is (no, of course i've never been guilty of this :P)

[silk pine]

that's a noble POV but I think people would be angry with in practice 😛

ie "i know it's not perfect but i don't wanna be punished until I can fix it 😭"

but, i'm not arguing against the behavior, i think it makes sense for there to be a mechanic where you can cache a failure (linting is a great example) (and ldd for checking static builds)

[dark yoke]

mm, i get that, i just think it's a bit weird to explain (in future docs) why some steps might "succeed" (and get their little tick in dagger cloud, etc), but don't end up cached

[silk pine]

succeed because exit-status==2 and 2 was one of the expected exits?

[dark yoke]

mm

yeah

out of curiosity - do you think this is a blocker for the feature?

[silk pine]

i think i'd still expect a red tick for that tbh. but i might be missing context from earlier in the thread

[dark yoke]

mostly because of this

if we have to not cache fails, i'm not sure how we actually do that

[silk pine]

i just think it's a bit of a pandora's box - feels like there's a decision matrix hidden in here to bikeshed, for bikeshedding's sake

[dark yoke]

😢 fair enough

😄 let the bikeshedding begin 🎉

[silk pine]

just my 2c! if it makes sense to everyone else i'm not blocking 😛

[dark yoke]

mmmm i think i would also expect a red tick or something

but i don't think it's a red cross

like it's this weird new third state

"this failed, but we continued onwards anyways"

although... i'm not sure i neccessarily know if I agree with the idea that it failed.
i could imagine writing e2e tests in dagger, and running commands that I expect to fail - e.g. our own integration tests. technically, when the dagger call "fails", it's doing the right behavior, it's a success (cloud's visualization of this atm is kinda odd, because we see a bunch of red spans, but we expect those spans to be red, but there's no way of indicating that)

i guess what i'm saying is i don't think "non-zero exit code == failure" and "zero exit code == success"

[silk pine]

yep - right now that's handled by the parent span. if the child span failed, and that was not expected, the parent span interprets that and bubbles up the failure. if it was expected, the parent span succeeds, and the failed span will still be failed, but it won't be bubbled up, since the parent span covers for it

(brb doctor)

[dark yoke]

(take your time, I'm gone for the day now, birthday dinner time 🥳)

[flint pendant]

Belated happy birthday to you! (Happy birthday to you... happy birthday to you!)

[flint pendant]

In relation to https://github.com/dagger/dagger/issues/8141

Currently I have no qualm with non-zero exit codes still being considered a failure. My main issue is that I cannot make use of that failure, I cannot dig into it at all. Not without using workarounds like this

Speaking literally of my use-case (and I welcome anyone to throw in a contradictory use-case) I am happy for it to still be a failure, but I need to be able to look at that failure.

For instance, if I use PHPCBF (PHP Code Beautifier and Formatter) exit code 1 is given when code needed formatting and it successfully formatted it. I need to be able to continue from that exit code and export the code it formatted.

[wide notch]

I think the key thing, which perhaps is another feature is to be able to retrieve the exit code from the last withExec() call.

[silk pine]

I think you can already do this, but it's language-dependent - in Go for example you can cast the error to a *dagger.ExecError and it'll have the exit code as a field

in Python/TS there's probably an exception with the same info

[dark yoke]

mm, if we make it stop throwing an error for specified exit codes though, we need another way of grabbing this 🤔

[silk pine]

yeah - we used to have an exitCode API but it was removed when we realized its only possible return value was 0 😂

which your feature would fix!

[wide notch]

yeah, the PHP SDK also has the same exception handling, however this PR means there won't be an exception/error so we won't get the code :p

[dark yoke]

i'll make sure to add exitCode to container in that pr - i've added a note 😆

[twin vigil]

@flint pendant @dark yoke thinking about this in the context of https://github.com/dagger/dagger/issues/8421.

Does our current cookbook stopgap for handling exit codes could potentially resolve https://github.com/dagger/dagger/issues/8141 ? It seems right to me to merge Justin's PR but unless I'm missing something the initial use-case from this thread is different than what the PHP SDK needs, correct?

[dark yoke]

Yes it does - but it's only a stopgap, and needs a proper fix

[twin vigil]

ok, seems to that the PHP SDK use-case is different than the OP's need, that's why I was getting confused

[wooden brook]

Like bash ?$

Good evening daggernauts 🫡 happy choos day 🚆 🚉

[twin vigil]

@silk pine I was reading about your take on the type Error API change. I really like the idea and I think it'll allow us to be way more expressive while setting and handling errors. I have two questions / observations about the current API.

1- Should error have a JSON type similarly to returnValue so we can eventually assign dynamic types to that?
2- How does this unblock us for the OP's use-case in this thread? Since now functions can return richer error type but from the CLI perspective we'll still have to deal with making the dagger call fail with some status code, and still be able to return something from it.

One thing that we were speaking with @jolly nova yesterday was the fact that it's not mandatory for us to necessarily fail with a status code != 0. In a similar way which GraphQL didn't comply with the HTTP / REST standard, we think it's ok to be a bit more disruptive here while giving the flexibility to the user to express what they need

[silk pine]

1 - Wasn't planning on that, because at that point we would need to keep track of two return types, which starts to feel a little complicated, especially if there are multiple types of errors an API author may want to return. We can get away with the JSON representation for returnValue because we know what type the function returns. For my proposal there's currently just a single blessed Error type which we could possibly extend to support error hierarchies and attaching arbitrary data, so my main question is whether that's enough to satisfy this 'error reports' use case.
2 - Hmm, well it wasn't really intended to solve this specific issue - my goal was just to have answer at all for how functions can return errors, since that's currently missing completely leaving us with awful exec /runtime: exit status 2 errors. But, provided there's some way to get structured data out of e.g. a failed go test (which might require Justin's proposal to support chaining from failed commands), you would be able to take that and turn it into an Error hierarchy. At that point it wouldn't be "I successfully returned a test report", it'd be "I failed, and here's the failure report"

So yeah, the big question is whether that's enough to cover all these use cases (lint + test, ???). Personally, I think it'd be an uphill battle to "successfully return a failed report" - I think you'd still want a nonzero exit status from call for example. Seems like anything else would be playing wack-a-mole and manually adding early exits, or mistakenly getting green Actions runs that emit failed reports, etc.

The other advantage of having a single Error type is it seems like it'd be much easier to work with in the UI and in code, as opposed to arbitrary error types or report types

The trade-off would be that if we support adding arbitrary data to it, you'll be duck-typing when you pull it back out

[twin vigil]

yes, I like that. Still thinking how we can support returning both errors + custom types / objects. As Justin mentioned in the issue, in Go it makes sense because errors are values. In languages where exceptions are thrown, the return values will have to be part of the error somehow in order to get them

[twin vigil]

that's what devs do in their languages in any case, right?

[silk pine]

well, if a module were able to return its own custom types (like test/lint reports or its own custom error implementations) it'd be strongly typed the whole time

maybe we could have an Error interface?

not sure if it's worth it 🤷‍♂️

[twin vigil]

but modules can return custom types? 🤔. As long as they're dagger serializable?

[silk pine]

sorry, meant custom error types there

[twin vigil]

got it, but I'd assume that we'd still want to generalize them to their "abstract" type? i.e in Go the SDK generated function will probably just return error, correct?

non "error as values" languages can catch the concrete type I assume, but that's a from of type casting basically...

:typescript: 🤣

[silk pine]

yeah I think in Go we'd make sure *Error implements error, and do the analogous thing for exception-based languages. So in Go you'd attempt a cast to *Error and start inspecting from there. With interfaces, I'm not quite sure how it'd work - you might have to cast it to *Error and then try .AsFooError() I guess? Feels like a lot of extra hops. (Also, don't even want to think about the case where you get a *Error but then msg, err2 := err.Message(ctx) fails 💀)

tbh I hate the pattern of catching/handling various error types

it's like a janky API within an API

so yeah as you can see we've quickly exceeded the scope of my bikeshed/proposal so far 😛

the MVP for me is just improving on the exec /runtime: exit status 2 errors

[silk pine]

we'd probably want to at least pre-fetch the message field - some sort of special-casing. I think we'd have to in order to even implement error, since it's Error() string - can't error or use ctx there

maybe that's where GraphQL error responses come in ... somehow

[twin vigil]

yes, let me think about that a bit more 🤔

[flint pendant]

The initial cookbook workaround is part of what we want, this is my current workaround.

Basically, on a non-zero exit code, we need:

1. A way to view the [result](#1275216033862647890 message) of the last withExec
2. A way to continue using the container if the result is acceptable.

#8466 solves 2. But it does not solve 1.

[dark yoke]

upstream has merged, so i'm gonna get this one ready to go today - i'm gonna do both parts ❤️

[twin vigil]

IIUC with #8466 you could also achieve 1, correct?

I'm imagining something like this:

func (m *Lala) Test() {

    ctx := context.Background()
    c := dag.Container().From("alpine").
        WithExec([]string{"sh", "-c", "touch foo.txt && exit 1"}, dagger.ContainerWithExecOpts{ExitCodes: []int{1}})

    ec, err := c.ExitCode(ctx)

    if err != nil {
        return nil, err
    }
    // do whatever with the exit code and continue the pipeline here.

    c.WithExec([]string{"do", "something", "else"})

}

[ember glacier]

Does that mean if a WIthExec fails with a non-zero exit code and the particular step was supposed to generate a file, it's going to be there?

[flint pendant]

As long as the exit code you wanted was specified in withExec(..., validExitCodes: ... I believe that is the idea

[flint pendant]

The dagger internals are a a bit of an unknown to me, so I read the tests to understand what behaviour is implemented.

What I don't see in the tests, is if I still get stdout and stderr. Would these still print out as normal?

It's not a blocker for the PR, @dark yoke has done amazing work and I'm excited to see it go in! Being able to get the exit code does fix issue #8141.

[dark yoke]

yes, these would appear as normal