Comments (14)
Yes, I can catch decoding errors at the call site like this, but that will lead to a lot of duplication.
This seems like a perfect fit for middleware conceptually, where other metrics and instrumentation can easily be added for all endpoints without needing the call sites to explicitly handle those cases.
Consider this a feature request to consider having centralized handling of decoding errors 🙏
And thanks for all your efforts in making this library ❤️
from swift-openapi-generator.
These are no doubt important questions for you to answer as a library author. From my perspective, any answer to those questions would satisfy my need to add instrumentation and alerting to api response decoding errors.
from swift-openapi-generator.
If it's just instrumentation that users are after, should we consider adding "actual" instrumentation to the runtime library, i.e. by depending on Swift Metrics and emitting metrics for these sorts of things with dimensions for the operationID and the kind of error?
- Comprehensive metrics would be great, but much broader scope compared to my near-term needs
- Ideally comprehensive metrics would be pluggable like
ClientTransport
currently is to:
a. avoid the cost of the swift-metrics dependency for users who don't need it
b. allow for alternative metrics implementations
Also I consider fine-grained metrics (number of requests, failure #s, timing data histograms, etc) to be fairly distinct from and very differently actionable compared to decoding errors where a server is returning non-conformant responses. So I can see some benefit in having a unified metrics solution but that's not necessary for this level of instrumentation.
from swift-openapi-generator.
Hi @jpsim,
you're right that a middleware won't be the solution here, as transports and middlewares still work with generic HTTP requests and responses, and only the type-safe layer closest to your code turns them into values of your generated types.
If you want to e.g. emit metrics for specifically decoding errors, you can catch ClientError
, which, as its underlyingError
will have the original DecodingError
thrown by JSONDecoder
. I'd suggest logging it together with the causeDescription
.
So, in code, you could do something like:
@main struct HelloWorldURLSessionClient {
static func main() async throws {
let client = Client(serverURL: URL(string: "http://localhost:8080/api")!, transport: URLSessionTransport())
do {
let response = try await client.getGreeting()
print(try response.ok.body.json.message)
} catch let error as ClientError where error.underlyingError is DecodingError {
print("The reason for this request failing was the following decoding error: \(error)")
// Increment your metric here.
// Rethrow the original error.
throw error
}
}
}
from swift-openapi-generator.
Thanks for the kind words 🙂
If we were to add a middleware at the suggested layer, do you have ideas of what the signature could be?
Thinking out-loud here, but if we based it on the raw HTTP one:
protocol Middleware: Sendable {
func intercept(_ input: Sendable, operationId: String, next: (Sendable) async throws -> Sendable) async throws -> Sendable
}
The input/output values would be of the Operations.MyOperation.{Input,Output}
types.
Would that be useful? I wonder if the as?
casting you'd need to do in the body of it would actually be useful/acceptable.
Opinions/ideas welcome here, also cc @simonjbeaumont.
from swift-openapi-generator.
Could we keep the type information by using generic parameters?
func intercept<Input: Sendable, Output: Sendable>(
input: Input,
operationId: String,
next: (Input) async throws -> Output
) async throws -> Output {
// Middleware could transform or log input, output or errors here
return try await next(input)
}
from swift-openapi-generator.
The Client type needs to have an array of these middlewares that all run on every request to every operation. But these Input/Output types are per-operation.
Today, we have a property var middlewares: [any ClientMiddleware]
on the underlying client. In my less-than-ideal scenario, that would be var middlewares: [any Middleware]
, for example.
What would that array signature be in your example? How would the Client code iterate through them?
from swift-openapi-generator.
I think with my suggestion, you could keep var middlewares: [any ClientMiddleware]
since the generic parameters are defined on the function, not the protocol, so it doesn't add any generic constraints to the protocol itself.
from swift-openapi-generator.
Sorry, I'm still not sure I understand how it'd actually work. Could you try to open a draft PR to the runtime library with your experimental change, and maybe that'll help me get it? 😇
from swift-openapi-generator.
Proof of concept here: apple/swift-openapi-runtime#99
I basically just took the shortest path I could find that would allow me to hook into decoding errors in a centralized place. I suspect you may want to go a different direction that may be a better fit conceptually for the project, but I'll leave that to you to decide.
from swift-openapi-generator.
Oh right, if you don't need the Input/Output when handling these errors, then yes we could totally do that.
A few thoughts:
- should this cover encoding errors as well, and errors such as a missing header when it was required?
- should the method itself throw, to allow you to handle the error and rethrow a different one (or the same one)? Maybe the method should return an error to ensure we always have something to rethrow after the handler returns.
- should the error be ClientError, which has more useful context, or just the underlying error?
Yeah let's discuss, this is definitely an interesting direction.
from swift-openapi-generator.
Oh right, if you don't need the Input/Output when handling these errors, then yes we could totally do that.
A few thoughts:
- should this cover encoding errors as well, and errors such as a missing header when it was required?
- should the method itself throw, to allow you to handle the error and rethrow a different one (or the same one)? Maybe the method should return an error to ensure we always have something to rethrow after the handler returns.
- should the error be ClientError, which has more useful context, or just the underlying error?
Yeah let's discuss, this is definitely an interesting direction.
Users can already catch and unpack the error today; the complaint wasn't that it wasn't possible, but that it was tedious to wrap every call like this. For this reason, I think that allowing the user to convert to a different error doesn't add much value. Same argument goes for whether this error is wrapped or unwrapped.
IIUC, the request is to instrument this particular error flow to log or collect metrics? In that case, any hook we provide should probably be purely an optional side-effect, so a void function.
I have a couple of questions of my own:
- If we're providing this as an optional function hook that users can provide, why make it specific to this error? Are there more places where such a handler could be beneficial.
- From an API perspective, could we position this as a middleware still (cf. a function hook), even if it means we need to do some things behind the scenes? E.g.
ErrorMiddleware(_ handler: (any Error) -> Void)
. - If it's just instrumentation that users are after, should we consider adding "actual" instrumentation to the runtime library, i.e. by depending on Swift Metrics and emitting metrics for these sorts of things with dimensions for the operationID and the kind of error?
from swift-openapi-generator.
All great points. If we say that this is purely a read-only action (no transforming of errors), then maybe something like this? (Naming to be bikeshed separately)
protocol ClientErrorDelegate: Sendable {
func requestSerializationDidThrow(_ error: ClientError) async throws
func transportOrMiddlewareDidThrow(_ error: ClientError) async throws
func responseDeserializationDidThrow(_ error: ClientError) async throws
}
And then you could optionally provide one of these on the Configuration
struct. WDYT?
from swift-openapi-generator.
We'd probably also provide a server variant:
protocol ServerErrorDelegate: Sendable {
func requestDeserializationDidThrow(_ error: ClientError) async throws
func transportOrMiddlewareDidThrow(_ error: ClientError) async throws // Not entirely sure how this one would work, TBD
func responseSerializationDidThrow(_ error: ClientError) async throws
}
// Edit: Actually, if this is Client/Server, we'd probably just add it as another parameter to the Client initializer/registerHandlers call. Configuration only has things that are the same for both today.
from swift-openapi-generator.
Related Issues (20)
- Realtime chat client HOT 4
- Automatically throw error if HTTP response represents a failure HOT 4
- Instance method 'getResponseBodyAsJSON(_:from:transforming:)' requires that 'HTTPBody' conform to 'Decodable' HOT 3
- Optional type alias issue HOT 2
- Advice regarding nullable refs? HOT 5
- Support generating values of type URL from JSON Schema HOT 5
- question: working with discriminators and inheritance on nested data HOT 8
- Configuring the Xcode plugin to generate the source code for checkin HOT 1
- Generated method names based on description rather than the API path HOT 4
- How to get the response headers from result HOT 1
- Switch over to the toolchain swift-format from building it in the CI script
- How to send invalid data with the client to test negative scenarios? HOT 4
- PlatformChecks emits error when compiling for MacCatalyst HOT 2
- Empty Dictionary fails to encode HOT 9
- How to calculate response length? HOT 6
- Issue with Executing Unit Tests via CLI with swift-openapi-generator added to the package. HOT 9
- registerHandlers should throw an error if serverURL doesn't match documentation HOT 2
- Allowing disabling percent encoding for some HTTP header fields HOT 10
- Default responses lead to invalid Swift code HOT 4
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from swift-openapi-generator.