Skip to content

Decode RequestError body#18

Closed
tunniclm wants to merge 4 commits intomasterfrom
add_requesterror_body
Closed

Decode RequestError body#18
tunniclm wants to merge 4 commits intomasterfrom
add_requesterror_body

Conversation

@tunniclm
Copy link
Copy Markdown
Collaborator

Add support for populating/decoding the body of a RequestError.

Requires: Kitura/SwiftyRequest#18


// Convert an Error to a RequestError, mapping HTTP error codes over if given a
// SwiftyRequest.RestError. Decorate the RequestError with Data if provided
fileprivate func constructRequestError(from error: Error, data: Data?) -> RequestError {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As discussed, would this be better as (uncompiled):

extension RequestError {
    init(from error: Error, data: Data?) {
        self = .clientErrorUnknown
        if let restError = error as? RestError {
            init(restError: restError)
        }
        if let data = data {
            do {
                // TODO: Check Content-Type for format, assuming JSON for now
                try init(requestError, bodyData: data, format: .json)
            } catch {
                // Do nothing, format not supported
            }
        }
    }
}

Perhaps in RequestErrorExtension.swift.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

After discussing this, we decided that this code was pretty specific to the file it was in and may not be generally useful and so were not sure if moving it out / to an init() is necessary.

@tunniclm
Copy link
Copy Markdown
Collaborator Author

Waiting for Kitura to release a tagged version with the required router features for this to work.

@djones6 djones6 force-pushed the add_requesterror_body branch from e2d1f43 to 169dc7c Compare May 2, 2018 13:06
@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #18 into master will increase coverage by 8.18%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #18      +/-   ##
==========================================
+ Coverage   71.96%   80.15%   +8.18%     
==========================================
  Files           2        2              
  Lines         132      131       -1     
==========================================
+ Hits           95      105      +10     
+ Misses         37       26      -11
Flag Coverage Δ
#KituraKit 80.15% <80%> (+8.18%) ⬆️
Impacted Files Coverage Δ
Sources/KituraKit/Client.swift 83.33% <80%> (+7.3%) ⬆️
Sources/KituraKit/RequestErrorExtension.swift 45.45% <0%> (+18.18%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d3b465f...169dc7c. Read the comment docs.

@djones6 djones6 mentioned this pull request May 2, 2018
@djones6
Copy link
Copy Markdown
Contributor

djones6 commented May 2, 2018

Replaced by #30 as KituraKit's current release process requires we merge to develop not master.

@djones6 djones6 closed this May 2, 2018
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.

4 participants