Repository navigation
Refuse to_s on an Option or a Result again, and release 0.9.1 - #81
Merged
Merged
Conversation
0.9.0 made Option#to_s render as inspect does, to keep a raise out of the rescue block that interpolates a wrapper into its log line. In a mass conversion of every model in an application, every site that built a string from a reader then wrote wrapper text into data with no exception: a UNIQUE identity column, a hostname, a hashed auth token and a customer-facing page. Each of those raised under 0.8.3. The raise comes back as the same Errgonomic::SerializeError, so an existing rescue still matches, and its message now carries the remedy the log-line case needs: the value via the bounded inspect, that to_s is refused, and inspect for a log line or unwrap_or / expect! for the value. The bounded inspect moves into its own helper so the to_json refusal and this one name the value the same way.
The same reversal as for Option, with the same message shape: the value via a bounded inspect, that to_s is refused, and the spellings to reach for. Result had no bounded inspect helper, so one is added privately for the to_s refusal; the to_json and as_json refusals keep their fixed message.
Each reproduction from the 0.9.0 field reports is a test: join, interpolation, format and String() for both containers; a bare ERB <%= %> through ActionView, whose template error carries the refusal as its cause; and an Option or Result in a Hash key, which both the json gem and ActiveSupport's as_json stringify through to_s, so the raising to_s closes the key position that 0.9.0 opened. A None in a bare ERB tag renders as nothing rather than raising: the Rails integration's None answers nil?, and ActionView's output buffer skips a nil before it asks for to_s. The test records that as it is.
The changelog names the to_s reversal as a behavior change with an upgrading note for anyone who wrote against 0.9.0's rendering. The 0.9.0 section stays as history.
lutzcc1
previously approved these changes
Sep 10, 2026
This was referenced Sep 10, 2026
unwrap_err! passed the Ok's value as the exception message, so the message was the value's to_s. With to_s refused on an Option or a Result, Ok(Some(1)).unwrap_err! printed only the class name and its message raised. The message is now the value's inspect, bounded as the to_s refusal bounds it, and the error carries the value as unwrap!'s does.
The join test covered the same shape with different values, and the group_by test grouped bare Options rather than a converted model's reader. Both lines now appear as #75 writes them: a join of a Some and a None, and a group_by over a converted model's optional association on its way to JSON.
With ActiveSupport loaded, Hash#to_json reaches a key through as_json,
which refused an Option under 0.9.0 as well, so the two to_json
assertions in the hash-key test passed before the to_s change and did
not test it. They move to a child process that loads the json gem and
errgonomic alone, where 0.9.0 rendered {"Some(1)":2} and a raising to_s
refuses.
A case/in with several branches and none matching raises NoMatchingPatternError with the subject itself as its message, so on an Option or a Result the message refuses. The top-level printer then shows only the class name, and Minitest's reporter crashes before it names the test. The README says so and that the fix is the missing branch, and a test pins each of the three behaviors so the text cannot drift from them. The json-gem child test shares the new run_ruby helper.
Logger's own formatter inspects a message that is not a String, so logger.info(opt) renders. ActiveSupport::TaggedLogging interpolates the message once a tag is set, and Rails tags every production request with its request_id, so the same call passes in tests and raises in production. The README and the 0.9.1 upgrading note say to write logger.info(opt.inspect), and a test pins both loggers' behavior.
nz
force-pushed
the
2026-09-10-errgonomic-0.9.x-triage/to-s-raise
branch
from
September 10, 2026 18:31
3c8ff50 to
fd66b4f
Compare
nz
marked this pull request as ready for review
September 10, 2026 18:32
nz
enabled auto-merge
September 10, 2026 18:32
lutzcc1
approved these changes
Sep 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Errgonomic 0.9.1 makes
Option#to_sandResult#to_sraiseErrgonomic::SerializeErroragain, as 0.8.3 did, with a message that names the value and says what to call instead. Closes #80 and closes #75. The reviewer is asked to confirm one thing: #80 asks for anErrgonomic.to_s_behaviorflag, and this change declines it in favor of an unconditional raise, because the one case rendering serves, a log line inside arescue, is already served byinspect.What changes
A string built from an Option or a Result fails where it is built. String interpolation,
Array#join,format,String(), a bare ERB<%= %>and a Hash key on its way to JSON all reach the value throughto_s, and each raises rather than writingSome("...")orNoneinto a hostname, a UNIQUE column or a page. The message is one line:inspectis unchanged. Arescue Errgonomic::SerializeErrorwritten against 0.8.x still matches. The changelog carries an upgrading note for code written against 0.9.0's rendering, and the 0.9.0 section stays as history.How it works
Option::Any#to_sandResult::Any#to_sraise. Each builds its message from a privatebounded_inspect, the same 60-character bound theto_jsonrefusal on Option already used, so a record or a long payload does not bury the message.to_smethods pin the exact message for interpolation,join,format,String()and the bounded form.test/rails_test.rbcarry the reproductions from to_s rendering (0.9.0) turns the loudest failure class into silent data corruption — make it opt-in #80 and as_json/to_json refusal misses Option hash KEYS — wrapper ships to payloads silently (0.9.0 regression) #75:test_a_wrapper_refuses_to_become_a_string,test_a_bare_erb_tag_refuses_a_wrapperthrough ActionView, andtest_an_option_in_a_hash_key_refuses_to_serializethrough the json gem and ActiveSupport'sas_json.to_srefuses and what to call instead.Evidence
Every line from #80 and #75, run on 2026-09-10 against this branch with Ruby 3.4.9, ActiveSupport 8.1.3.1 and json 2.21.2. 0.9.0 is
mainat 43bbd0a.[Some("org"), Some("metrics")].join("/")Some("org")/Some("metrics")"#{None()}.us-east-1.example"None.us-east-1.example"data_#{Some("hot")}"data_Some("hot")format("%s", Some(1)),String(Some(1))Some(1)<%= Some("visible") %>Some("visible")ActionView::Template::Error{Some(1) => 2}.as_json{"Some(1)" => 2}JSON.generate({Some(1) => 2}){"Some(1)":2}group_by { Some/None }.to_json, json gem only{"None":[1,3],"Some(:even)":[2]}Some(1).inspectSome(1)Some(1)The #75 key position closes through
to_salone: the json gem's generator and ActiveSupport'sHash#as_jsonboth stringify a key withkey.to_s. Noto_stris added. One line in #75 did not reproduce as filed: with ActiveSupport loaded,{Some(1) => 2}.to_jsonalready raised on 0.9.0, because ActiveSupport 8.1 runs a key throughas_jsonbefore the generator sees it. The same call raises on this branch either way.Gates, all run locally in the branch's devshell on 2026-09-10:
bundle exec rubocop: 6 offenses, all present onmainat 43bbd0a and none on a changed line. See open items.bundle exec yard doctest: 174 runs, 436 assertions, 0 failures, 0 errors.bundle exec rake test: 132 runs, 464 assertions, 0 failures, 0 errors.bundle exec rake test:strict: 132 runs, 464 assertions, 0 failures, 0 errors.bundle exec rake: the three suites above, all passing.nix build .#errgonomic: builterrgonomic-0.9.1; the check phase ran the doctests.nix flake check --all-systems: all checks passed.CI on this branch ran
yard doctest,rake testandrake test:stricton Ruby 3.4 and passed on 2026-09-10 (run 34495870058).What is left open
Nonein a bare ERB<%= %>renders as nothing under the Rails integration, where aSomeraises.None#nil?answers true there, and ActionView's output buffer skips a nil before it asks forto_s. The test records that as it is. Recommendation: accept here. It is thenil?compromise the README names, notto_s, and a raise there means revisitingnil?.mainthat this branch does not touch:MetricsonOption#method_missing,Result#method_missingandoptional_dig, andStyle/DocumentationonErrgonomic::Rails. CI does not run rubocop. Recommendation: fix in their own commit outside this hotfix.Result#to_jsonandResult#as_jsonstill saycannot serialize an unwrapped Resultwithout naming the value, where Option's refusal names it. Recommendation: fix outside this hotfix.mainwithrake release.After review
Review on 2026-09-10 found three places where a raising
to_sreads badly and two test gaps. Following the driver's ruling that such seams are documented rather than patched, this branch now carries:Result#unwrap_err!on anOkbuilds its message from the value's boundedinspect, soOk(Some(1)).unwrap_err!printsSome(1)where it printed a bare class name and itsmessageraised. It was the only place in the gem that passed a value as an exception message. Changelog bullet under 0.9.1.case/inthat matches nothing on a wrapper looks like, a bareNoMatchingPatternErrorand a Minitest reporter crash with no test name, and that the fix is the missinginbranch or anelse. It also sayslogger.info(opt)raises under Rails'TaggedLoggingonce a tag such asrequest_idis set, so it can pass in tests and raise in production, and to writelogger.info(opt.inspect). The upgrading note names both, and a test pins each behavior the text describes.to_jsonassertions that already raised on 0.9.0 with ActiveSupport loaded are replaced by a test in a child process that loads only the json gem, where 0.9.0 rendered{"Some(1)":2}. The as_json/to_json refusal misses Option hash KEYS — wrapper ships to payloads silently (0.9.0 regression) #75 lines[Some(1), None()].join(",")and a converted model'sgroup_by(&:author).to_jsonare tests as the issue writes them.Gates after the fixes, 2026-09-10: rubocop 6 offenses, the same six;
yard doctest174 runs, 438 assertions;rake testandrake test:strict136 runs, 480 assertions each;nix build .#errgonomicandnix flake check --all-systemspass. CI passed on3c8ff50.