Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better response error handling #32

Merged
merged 5 commits into from
Nov 21, 2023
Merged

Better response error handling #32

merged 5 commits into from
Nov 21, 2023

Conversation

F0x1d
Copy link
Contributor

@F0x1d F0x1d commented Nov 15, 2023

Hi there. This pull request is about #17 and my comment there. I added PapyrusResponseError, which extends PapyrusError.

Now we can have something like this:

API:

@API
protocol GPT {
    @POST("images/generations")
    func generateImage(body: Body<GenerateImageRequestBody>) async throws -> GenerateImageResponse
}

Repository:

let body = ...
return try await gptApi.generateImage(body: body)

Some error handling, for example in ViewModel:

func handleError(_ error: Error) async {
    guard let error = error as? PapyrusError else { return }
    var message = error.message
    
    if let response = (error as? PapyrusResponseError)?.response,
       let responseError = try? response.decode(ErrorResponse.self, using: decoder) {
        message = responseError.error.message
    }
    
    self.error = message
}

I don't think this is the best solution for now. Waiting for you comments.

@F0x1d
Copy link
Contributor Author

F0x1d commented Nov 17, 2023

BTW, this way

... -> Response

methods will also throw PapyrusError. I think this is more intuitive. So, docs should be updated

@joshuawright11
Copy link
Owner

This is great @F0x1d ! I was thinking of doing things the same way.

How about we give an optional let request: Request? and let response: Response? to PapyrusError and we can use that error for everything (passing a request and response if one or both are available)?

@F0x1d
Copy link
Contributor Author

F0x1d commented Nov 17, 2023

I thought it would be strange to have the same error for something like Incorrect url and something like Unsuccessful status code, but, yeah, you are right, it makes life simpler

@F0x1d F0x1d changed the title Added PapyrusResponseError Better response error handling Nov 17, 2023
Copy link
Owner

@joshuawright11 joshuawright11 left a comment

Choose a reason for hiding this comment

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

Looks great 🎉

@joshuawright11 joshuawright11 merged commit b9bdc84 into joshuawright11:main Nov 21, 2023
2 checks passed
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.

2 participants