Skip to content

Commit 266d072

Browse files
stainless-app[bot]meorphis
authored andcommitted
fix: issue where we cannot mutate arrays on base model derivatives
array properties are now always recursively coerced into the desire type upon being set, instead of "almost always" hash key names are no longer unnecessarily translated when creating base models via hash coercion errors are now stored and re-thrown instead of being re-computed each property access fixed inconsistencies where sometimes `TypeError`s would be thrown instead of `ArgumentError`s, and vice versa
1 parent b78fa7c commit 266d072

File tree

19 files changed

+298
-110
lines changed

19 files changed

+298
-110
lines changed

lib/openai/errors.rb

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,28 @@ class Error < StandardError
99
end
1010

1111
class ConversionError < OpenAI::Errors::Error
12+
# @return [StandardError, nil]
13+
def cause = @cause.nil? ? super : @cause
14+
15+
# @api private
16+
#
17+
# @param on [Class<StandardError>]
18+
# @param method [Symbol]
19+
# @param target [Object]
20+
# @param value [Object]
21+
# @param cause [StandardError, nil]
22+
def initialize(on:, method:, target:, value:, cause: nil)
23+
cls = on.name.split("::").last
24+
25+
message = [
26+
"Failed to parse #{cls}.#{method} from #{value.class} to #{target.inspect}.",
27+
"To get the unparsed API response, use #{cls}[#{method.inspect}].",
28+
cause && "Cause: #{cause.message}"
29+
].filter(&:itself).join(" ")
30+
31+
@cause = cause
32+
super(message)
33+
end
1234
end
1335

1436
class APIError < OpenAI::Errors::Error

lib/openai/internal/type/array_of.rb

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,14 @@ def hash = [self.class, item_type].hash
6262
#
6363
# @param state [Hash{Symbol=>Object}] .
6464
#
65-
# @option state [Boolean, :strong] :strictness
65+
# @option state [Boolean] :translate_names
66+
#
67+
# @option state [Boolean] :strictness
6668
#
6769
# @option state [Hash{Symbol=>Object}] :exactness
6870
#
71+
# @option state [Class<StandardError>] :error
72+
#
6973
# @option state [Integer] :branched
7074
#
7175
# @return [Array<Object>, Object]
@@ -74,6 +78,7 @@ def coerce(value, state:)
7478

7579
unless value.is_a?(Array)
7680
exactness[:no] += 1
81+
state[:error] = TypeError.new("#{value.class} can't be coerced into #{Array}")
7782
return value
7883
end
7984

lib/openai/internal/type/base_model.rb

Lines changed: 76 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ def fields
6060
[OpenAI::Internal::Type::Converter.type_info(type_info), type_info]
6161
end
6262

63-
setter = "#{name_sym}="
63+
setter = :"#{name_sym}="
6464
api_name = info.fetch(:api_name, name_sym)
6565
nilable = info.fetch(:nil?, false)
6666
const = required && !nilable ? info.fetch(:const, OpenAI::Internal::OMIT) : OpenAI::Internal::OMIT
@@ -77,30 +77,61 @@ def fields
7777
type_fn: type_fn
7878
}
7979

80-
define_method(setter) { @data.store(name_sym, _1) }
80+
define_method(setter) do |value|
81+
target = type_fn.call
82+
state = OpenAI::Internal::Type::Converter.new_coerce_state(translate_names: false)
83+
coerced = OpenAI::Internal::Type::Converter.coerce(target, value, state: state)
84+
error = @coerced.store(name_sym, state.fetch(:error) || true)
85+
stored =
86+
case [target, error]
87+
in [OpenAI::Internal::Type::Converter | Symbol, nil]
88+
coerced
89+
else
90+
value
91+
end
92+
@data.store(name_sym, stored)
93+
end
8194

95+
# rubocop:disable Style/CaseEquality
96+
# rubocop:disable Metrics/BlockLength
8297
define_method(name_sym) do
8398
target = type_fn.call
84-
value = @data.fetch(name_sym) { const == OpenAI::Internal::OMIT ? nil : const }
85-
state = {strictness: :strong, exactness: {yes: 0, no: 0, maybe: 0}, branched: 0}
86-
if (nilable || !required) && value.nil?
87-
nil
88-
else
89-
OpenAI::Internal::Type::Converter.coerce(
90-
target,
91-
value,
92-
state: state
99+
100+
case @coerced[name_sym]
101+
in true | false if OpenAI::Internal::Type::Converter === target
102+
@data.fetch(name_sym)
103+
in ::StandardError => e
104+
raise OpenAI::Errors::ConversionError.new(
105+
on: self.class,
106+
method: __method__,
107+
target: target,
108+
value: @data.fetch(name_sym),
109+
cause: e
93110
)
111+
else
112+
Kernel.then do
113+
value = @data.fetch(name_sym) { const == OpenAI::Internal::OMIT ? nil : const }
114+
state = OpenAI::Internal::Type::Converter.new_coerce_state(translate_names: false)
115+
if (nilable || !required) && value.nil?
116+
nil
117+
else
118+
OpenAI::Internal::Type::Converter.coerce(
119+
target, value, state: state
120+
)
121+
end
122+
rescue StandardError => e
123+
raise OpenAI::Errors::ConversionError.new(
124+
on: self.class,
125+
method: __method__,
126+
target: target,
127+
value: value,
128+
cause: e
129+
)
130+
end
94131
end
95-
rescue StandardError => e
96-
cls = self.class.name.split("::").last
97-
message = [
98-
"Failed to parse #{cls}.#{__method__} from #{value.class} to #{target.inspect}.",
99-
"To get the unparsed API response, use #{cls}[#{__method__.inspect}].",
100-
"Cause: #{e.message}"
101-
].join(" ")
102-
raise OpenAI::Errors::ConversionError.new(message)
103132
end
133+
# rubocop:enable Metrics/BlockLength
134+
# rubocop:enable Style/CaseEquality
104135
end
105136

106137
# @api private
@@ -200,10 +231,14 @@ class << self
200231
#
201232
# @param state [Hash{Symbol=>Object}] .
202233
#
203-
# @option state [Boolean, :strong] :strictness
234+
# @option state [Boolean] :translate_names
235+
#
236+
# @option state [Boolean] :strictness
204237
#
205238
# @option state [Hash{Symbol=>Object}] :exactness
206239
#
240+
# @option state [Class<StandardError>] :error
241+
#
207242
# @option state [Integer] :branched
208243
#
209244
# @return [self, Object]
@@ -217,20 +252,23 @@ def coerce(value, state:)
217252

218253
unless (val = OpenAI::Internal::Util.coerce_hash(value)).is_a?(Hash)
219254
exactness[:no] += 1
255+
state[:error] = TypeError.new("#{value.class} can't be coerced into #{Hash}")
220256
return value
221257
end
222258
exactness[:yes] += 1
223259

224260
keys = val.keys.to_set
225261
instance = new
226262
data = instance.to_h
263+
viability = instance.instance_variable_get(:@coerced)
227264

228265
# rubocop:disable Metrics/BlockLength
229266
fields.each do |name, field|
230267
mode, required, target = field.fetch_values(:mode, :required, :type)
231268
api_name, nilable, const = field.fetch_values(:api_name, :nilable, :const)
269+
src_name = state.fetch(:translate_names) ? api_name : name
232270

233-
unless val.key?(api_name)
271+
unless val.key?(src_name)
234272
if required && mode != :dump && const == OpenAI::Internal::OMIT
235273
exactness[nilable ? :maybe : :no] += 1
236274
else
@@ -239,9 +277,10 @@ def coerce(value, state:)
239277
next
240278
end
241279

242-
item = val.fetch(api_name)
243-
keys.delete(api_name)
280+
item = val.fetch(src_name)
281+
keys.delete(src_name)
244282

283+
state[:error] = nil
245284
converted =
246285
if item.nil? && (nilable || !required)
247286
exactness[nilable ? :yes : :maybe] += 1
@@ -255,6 +294,8 @@ def coerce(value, state:)
255294
item
256295
end
257296
end
297+
298+
viability.store(name, state.fetch(:error) || true)
258299
data.store(name, converted)
259300
end
260301
# rubocop:enable Metrics/BlockLength
@@ -430,7 +471,18 @@ def to_yaml(*a) = OpenAI::Internal::Type::Converter.dump(self.class, self).to_ya
430471
# Create a new instance of a model.
431472
#
432473
# @param data [Hash{Symbol=>Object}, self]
433-
def initialize(data = {}) = (@data = OpenAI::Internal::Util.coerce_hash!(data).to_h)
474+
def initialize(data = {})
475+
@data = {}
476+
@coerced = {}
477+
OpenAI::Internal::Util.coerce_hash!(data).each do
478+
if self.class.known_fields.key?(_1)
479+
public_send(:"#{_1}=", _2)
480+
else
481+
@data.store(_1, _2)
482+
@coerced.store(_1, false)
483+
end
484+
end
485+
end
434486

435487
class << self
436488
# @api private

lib/openai/internal/type/boolean.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,20 @@ def self.==(other) = other.is_a?(Class) && other <= OpenAI::Internal::Type::Bool
3131
class << self
3232
# @api private
3333
#
34+
# Coerce value to Boolean if possible, otherwise return the original value.
35+
#
3436
# @param value [Boolean, Object]
3537
#
3638
# @param state [Hash{Symbol=>Object}] .
3739
#
38-
# @option state [Boolean, :strong] :strictness
40+
# @option state [Boolean] :translate_names
41+
#
42+
# @option state [Boolean] :strictness
3943
#
4044
# @option state [Hash{Symbol=>Object}] :exactness
4145
#
46+
# @option state [Class<StandardError>] :error
47+
#
4248
# @option state [Integer] :branched
4349
#
4450
# @return [Boolean, Object]

0 commit comments

Comments
 (0)