Skip to content

swift: add response interfaces#261

Closed
rebello95 wants to merge 2 commits intomasterfrom
swift-add-response-interfaces
Closed

swift: add response interfaces#261
rebello95 wants to merge 2 commits intomasterfrom
swift-add-response-interfaces

Conversation

@rebello95
Copy link
Copy Markdown
Contributor

Adds Swift interfaces for responses within Envoy. These should be at parity with the interfaces being added for Kotlin in #260.

Resolves #118.

Doc for reference: https://docs.google.com/document/d/1N0ZFJktK8m01uqqgfDRVB9mpC1iEn9dqkQaa_yMn_kE/edit#heading=h.i6ky65xaa9va

Signed-off-by: Michael Rebello mrebello@lyft.com

Adds Swift interfaces for responses within Envoy. These should be at parity with the interfaces being added for Kotlin in #260.

Resolves #118.

Doc for reference: https://docs.google.com/document/d/1N0ZFJktK8m01uqqgfDRVB9mpC1iEn9dqkQaa_yMn_kE/edit#heading=h.i6ky65xaa9va

Signed-off-by: Michael Rebello <mrebello@lyft.com>
@rebello95 rebello95 added this to the v0.2 "Primo" milestone Jul 16, 2019
/// Error representing cases when no response was received from the server.
/// I.e., the client went offline or became disconnected.
@objcMembers
public final class NetworkError: NSObject, Swift.Error {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hm, more of a platform question:
It is a swift convention to return an error and let the caller handle the error that way, correct?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is a swift convention to return an error and let the caller handle the error that way, correct?

Are you referring to doing this instead of throwing?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yeah

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The benefit of returning an error is that it can be typed. Swift doesn’t allow you to specify which errors can be thrown by a function, so consumers would have to catch all errors and cast. Also throwing wouldn’t work for async

Signed-off-by: Michael Rebello <mrebello@lyft.com>
@goaway
Copy link
Copy Markdown
Contributor

goaway commented Jul 16, 2019

Cross-posting from here:
#260 (comment)

This echoes my question on the Request interfaces, but have we thought at all about how might expose response data for streamed processing?

This is standard in most HTTP library interfaces, and generally processing/handling of responses tends to be something that can make progress in an online fashion faster than bytes come over the network (i.e. you shouldn't wait to start processing until all bytes have arrived).

@rebello95
Copy link
Copy Markdown
Contributor Author

Will comment on that PR so we can discuss, thanks for reviewing! 😄

rebello95 added a commit that referenced this pull request Jul 19, 2019
Adds Swift interfaces for responses within Envoy. These should be at parity with the interfaces being added for Kotlin in #265.

Replaces #261.

Resolves #118.

Signed-off-by: Michael Rebello <mrebello@lyft.com>
@rebello95
Copy link
Copy Markdown
Contributor Author

Replaced by: #273

@rebello95 rebello95 closed this Jul 19, 2019
@rebello95 rebello95 deleted the swift-add-response-interfaces branch July 19, 2019 01:01
rebello95 added a commit that referenced this pull request Jul 22, 2019
Adds Swift interfaces for responses within Envoy. These should be at parity with the interfaces being added for Kotlin in #265.

Replaces #261.

Resolves #118.
Also resolves #247.

Signed-off-by: Michael Rebello <me@michaelrebello.com>
jpsim pushed a commit to envoyproxy/envoy that referenced this pull request Nov 28, 2022
Adds Swift interfaces for responses within Envoy. These should be at parity with the interfaces being added for Kotlin in envoyproxy/envoy-mobile#265.

Replaces envoyproxy/envoy-mobile#261.

Resolves envoyproxy/envoy-mobile#118.
Also resolves envoyproxy/envoy-mobile#247.

Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit to envoyproxy/envoy that referenced this pull request Nov 29, 2022
Adds Swift interfaces for responses within Envoy. These should be at parity with the interfaces being added for Kotlin in envoyproxy/envoy-mobile#265.

Replaces envoyproxy/envoy-mobile#261.

Resolves envoyproxy/envoy-mobile#118.
Also resolves envoyproxy/envoy-mobile#247.

Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: JP Simard <jp@jpsim.com>
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.

ios: Unary HTTP request interfaces

3 participants