Skip to content

Allow custom client error handling based on response media type for non 2xx status codes - #88

Open
lucianogamberale wants to merge 7 commits into
release-candidatefrom
85-allow-custom-client-error-handling-based-on-response-media-type-for-non-2xx-status-codes
Open

Allow custom client error handling based on response media type for non 2xx status codes#88
lucianogamberale wants to merge 7 commits into
release-candidatefrom
85-allow-custom-client-error-handling-based-on-response-media-type-for-non-2xx-status-codes

Conversation

@lucianogamberale

Copy link
Copy Markdown

No description provided.

@codecov

codecov Bot commented May 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.72%. Comparing base (4d44131) to head (c3bbd81).

Additional details and impacted files
@@                  Coverage Diff                  @@
##           release-candidate      #88      +/-   ##
=====================================================
+ Coverage              99.70%   99.72%   +0.02%     
=====================================================
  Files                     54       57       +3     
  Lines                   3676     3975     +299     
=====================================================
+ Hits                    3665     3964     +299     
  Misses                    11       11              
Flag Coverage Δ
unit 99.72% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jvanecek
jvanecek previously approved these changes May 22, 2026
]

{ #category : 'private' }
RESTfulAPIClientTest >> applicationJSONErrorMessageBuilder [

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the prefix builder makes it confusing. Do you have an alternative?

@lucianogamberale lucianogamberale May 26, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the feedback! You are completely right; using the "Builder" suffix here is confusing since the method simply returns a BlockClosure and doesn't implement the Builder pattern.

To better align with Smalltalk idioms and the repository's naming conventions, I came up with these three alternatives:

  • #applicationJSONErrorMessageBlock: Since the method returns a closure, using the Block suffix is the most standard and direct Smalltalk convention.
  • #applicationJSONErrorMessageProvider: This aligns perfectly with the framework's existing terminology, similar to how httpClientProvider or currentTimeProvider are used.
  • #applicationJSONErrorParser: This describes the actual mechanical action happening inside the closure (parsing the JSON to extract the message).

I personally lean towards ...Provider as it feels very native to the current codebase, but I'd love to know which one you think fits best with the team's style so I can update it!

Comment thread source/Superluminal-RESTfulAPI-Tests/RESTfulAPIClientTest.class.st Outdated
Comment thread source/Superluminal-RESTfulAPI/HttpErrorMessageBuilder.class.st Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants