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

Add error handling for API errors #26

Merged

Conversation

czuria1
Copy link
Contributor

@czuria1 czuria1 commented Nov 2, 2020

Resolves #7

@Jakub-Vacek
Copy link
Collaborator

Nicely done 🤩👍

@Jakub-Vacek Jakub-Vacek added the hacktoberfest-accepted Approved PR's according to Hacktoberfest label Nov 2, 2020

import Foundation

enum Result<T> {
Copy link
Contributor

Choose a reason for hiding this comment

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

You might just use Swift.Result instead of introducing a new enum

case emptyResponse
case invalidDate
case configurationNotFound
enum ApiError: String, Error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use LocalizedError.errorDescription for this purpose, instead of rawValue

pipelinerService.getProjectName(baseUrl: baseUrl, projectId: projectId, token: token) { (result) in
switch result {
case .success(let projectName):
print(projectName)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please keep configurations update and reload here

WidgetCenter.shared.reloadAllTimelines()

} else {
savedBaseUrl = "not found"
Copy link
Contributor

Choose a reason for hiding this comment

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

savedBaseUrl seems to be unused anymore. Please remove it

let project = try JSONDecoder().decode(Project.self, from: data)

guard let data = try? await(httpService.getData(request: request)) else {
throw ApiError.emptyResponse
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't drop the original error, because it might include relevant info for issue debugging.

do {
return try gitLabService.getProjectName(baseUrl: baseUrl, projectId: projectId, token: token)
let projectName = try gitLabService.getProjectName(baseUrl: baseUrl, projectId: projectId, token: token)
completion(.success(projectName))
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the reason to make this asynchronous?

…dling

# Conflicts:
#	Pipeliner/ContentView.swift
#	Pipeliner/GitHub/GitHubService.swift
#	Pipeliner/PipelinerService.swift
@Jakub-Vacek Jakub-Vacek changed the base branch from master to feat/error-handling December 23, 2020 07:13
@Jakub-Vacek Jakub-Vacek merged commit 9720709 into DXHeroes:feat/error-handling Dec 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Approved PR's according to Hacktoberfest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve error handling
3 participants