Skip to content

Refine TypedNetwork API visibility: expose APIClient, internalize MiddlewareChain#5

Open
maumar2000 wants to merge 1 commit into
mainfrom
codex/audit-and-refactor-api-visibility-in-typednetwork
Open

Refine TypedNetwork API visibility: expose APIClient, internalize MiddlewareChain#5
maumar2000 wants to merge 1 commit into
mainfrom
codex/audit-and-refactor-api-visibility-in-typednetwork

Conversation

@maumar2000
Copy link
Copy Markdown
Owner

Motivation

  • Make the package safe and ergonomic for downstream consumers by explicitly exposing the primary client surface (APIClient) while keeping middleware orchestration internals hidden.

Description

  • Promote APIClient to public and make its initializer and send(_:) method public, and reduce accidental exposure by changing MiddlewareChain from public to internal (removed public from the type and its init). Files modified: Sources/TypedNetwork/APIClient.swift, Sources/TypedNetwork/Middleware.swift.

Testing

  • Ran swift build and swift test; both succeeded (swift test ran the suite and all tests passed).

Codex Task

@maumar2000
Copy link
Copy Markdown
Owner Author

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 32e9eaeb86

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

}

func send<E: Endpoint>(_ endpoint: E) async throws -> E.Response {
public func send<E: Endpoint>(_ endpoint: E) async throws -> E.Response {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Handle non-2xx responses for Never endpoints

Making send public exposes the existing non-2xx path to downstream clients whose endpoints use the protocol default Failure == Never. In that common case, any 4xx/5xx response calls the default Endpoint.mapError, which unconditionally fatalErrors instead of throwing NetworkError, so a public API request can crash the app on an ordinary server error unless every consumer defines a custom failure mapper.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

@codex address that feedback

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Summary

  • Updated APIClient.send(_:) to guard the non-2xx path for endpoints whose Failure is Never, so it no longer calls mapError (which previously triggered a fatalError via the default implementation). It now throws NetworkError.endpoint with an internal fallback error instead.
  • Added a regression test covering a Failure == Never endpoint receiving a 500 response, verifying that the client throws instead of crashing.
  • Committed changes on branch work with commit message: Handle non-2xx responses for Never-failure endpoints.

Testing

  • swift test
  • git status --short
  • git commit -m "Handle non-2xx responses for Never-failure endpoints"

Additionally, I created the follow-up PR message via make_pr summarizing the fix and validation.

View task →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant