Skip to content

Fix valid_provider! raising ArgumentError instead of InvalidConfigError#964

Merged
dersam merged 1 commit into
mainfrom
juni/06-19-fix_valid_provider_raising_argumenterror_instead_of_invalidconfigerror
Jun 22, 2026
Merged

Fix valid_provider! raising ArgumentError instead of InvalidConfigError#964
dersam merged 1 commit into
mainfrom
juni/06-19-fix_valid_provider_raising_argumenterror_instead_of_invalidconfigerror

Conversation

@juniper-shopify

@juniper-shopify juniper-shopify commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Problem

Both Chat::Config#valid_provider! and Agent::Config#valid_provider!
raise ArgumentError when given an invalid provider name, despite their
doc comments explicitly promising InvalidConfigError:

# Note: this method will return the name of a valid provider
# or raise an `InvalidConfigError`.
def valid_provider!
  # ...
  raise ArgumentError, "'...' is not a valid provider..."
end

This means any caller writing rescue InvalidConfigError — following
the documented contract — will not catch invalid provider errors. The
exception escapes as an unhandled ArgumentError instead of flowing
through the ConfigError hierarchy like every other config validation.

Fix

Changed both methods to raise InvalidConfigError (which they inherit
from Cog::Config via the class hierarchy). This is consistent with:

  • Chat::Config#valid_api_key! (line 124), which already raises
    InvalidConfigError for missing API keys
  • Map::Config#valid_parallel! (line 89)
  • Config#working_directory (lines 297–298)
  • All Config#validate! overrides across the cog tree

Updated the corresponding tests in both chat/config_test.rb and
agent/config_test.rb to assert Cog::Config::InvalidConfigError
instead of ArgumentError.

## Problem

Both `Chat::Config#valid_provider!` and `Agent::Config#valid_provider!`
raise `ArgumentError` when given an invalid provider name, despite their
doc comments explicitly promising `InvalidConfigError`:

```ruby
# Note: this method will return the name of a valid provider
# or raise an `InvalidConfigError`.
def valid_provider!
  # ...
  raise ArgumentError, "'...' is not a valid provider..."
end
```

This means any caller writing `rescue InvalidConfigError` — following
the documented contract — will not catch invalid provider errors. The
exception escapes as an unhandled `ArgumentError` instead of flowing
through the `ConfigError` hierarchy like every other config validation.

## Fix

Changed both methods to raise `InvalidConfigError` (which they inherit
from `Cog::Config` via the class hierarchy). This is consistent with:

- `Chat::Config#valid_api_key!` (line 124), which already raises
  `InvalidConfigError` for missing API keys
- `Map::Config#valid_parallel!` (line 89)
- `Config#working_directory` (lines 297–298)
- All `Config#validate!` overrides across the cog tree

Updated the corresponding tests in both `chat/config_test.rb` and
`agent/config_test.rb` to assert `Cog::Config::InvalidConfigError`
instead of `ArgumentError`.

## Files changed

- `lib/roast/cogs/chat/config.rb` — line 70
- `lib/roast/cogs/agent/config.rb` — line 54
- `test/roast/cogs/chat/config_test.rb` — line 53
- `test/roast/cogs/agent/config_test.rb` — line 34

juniper-shopify commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

@dersam dersam merged commit dcd44ba into main Jun 22, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants