apple / apple/swift-argument-parser

ArgumentParser spuriously exits with a zero status on certain errors

Open
#269 6 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Swift
Stars
3.8k
Forks
411
Avg merge
7d 13h
Merged PRs (30d)
17

Description

This package uses the `errorCode` property of `CustomNSError` errors as the process exit status.

```swift
// MessageInfo.swift:109
case let error as CustomNSError:
self = .other(message: error.localizedDescription, exitCode: Int32(error.errorCode))
```

The purpose of `errorCode` is merely to differentiate error cases within the same error domain; it is not appropriate to use it as an exit status. The process exit status has a limited range (either 8 or 32 bits depending on the syscall used to retrieve it); additional bits get silently truncated. As a result, when the classic `wait`/`waitpid` syscalls are used, `ArgumentParser` processes may appear to exit with a zero status (EXIT_SUCCESS) even if they failed.

Additionally, an `errorCode` that doesn't fit in an `Int32` leads to a fatal runtime error.

The broken behavior was introduced in #244. One easy way to fix this is to revert that change.

One way to correctly implement #230 is to define a custom mix-in error protocol with an explicit property for exit codes that is kept distinct from `errorCode`.

**ArgumentParser version:** `0.3.2`
**Swift version:** Latest stable

### Checklist
- [X] If possible, I've reproduced the issue using the `main` branch of this package
- [X] I've searched for [existing GitHub issues](https://github.com/apple/swift-argument-parser/issues)

### Steps to Reproduce

```swift
import Foundation
import ArgumentParser

enum MyError: Int, CustomNSError {
case foo = 256
case bar = 10_000_000_000

static var errorDomain: String { "MyError" }
var errorCode: Int { self.rawValue }
var errorUserInfo: [String : Any] { [:] }
}

struct Command: ParsableCommand {
@Flag
var trap: Bool = false

func run() throws {
throw trap ? MyError.bar : MyError.foo
}
}

Command.main()
```

### Expected behavior

I expect `ArgumentParser.main` to exit normally with a non-zero status whenever `run` throws an error it doesn't otherwise handle.

On the bash/fish/zsh prompt, I expect to see:

```
$ swift run test
$ echo $?
1 # Or some other non-zero value

$ swift run test --trap
$ echo $?
1 # Or some other non-zero value
```

### Actual behavior

Depending on the `errorCode` value, the subprocess appears to exit with a zero status, or exits with a signal instead.

With the bash/zsh installations on default macOS, and fish installed from homebrew, I get:

```
$ swift run test
$ echo $?
0
$ swift run test --trap
Swift/Integers.swift:3550: Fatal error: Not enough bits to represent the passed value
Illegal instruction: 4
```

Exiting with (what appears to be) a zero exit code when the process did not succeed is a very serious problem.

Shells (and other processes) that use the newer `waitid` syscall can get access to the full 32 bits of the exit code, so they'd report a nonzero code here. However, all shells I tried so far are still using the older `wait`/`waitpid` syscalls; we ought to be very conservative about what values we pass to `exit`.

Contributor guide

Open the contributing guide

Research direction

Start in MessageInfo.swift around line 109 and review the change introduced by #244, then run the supplied CustomNSError reproduction with and without --trap. Check how the existing discussion around #230 relates to the desired behavior. Done means thrown errors consistently produce a non-zero process status without a fatal overflow trap.

Written by the indexing model from the issue text.

Assessment

Tech stack
swift
Domain
cli
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.