parse-community / Parse-Swift

The Swift SDK for Parse Platform (iOS, macOS, watchOS, tvOS, Linux, Android, Windows)
https://parseplatform.org
MIT License
308 stars 69 forks source link

fix: check for ParseError first after server response #332

Closed cbaker6 closed 2 years ago

cbaker6 commented 2 years ago

New Pull Request Checklist

Issue Description

Currently tries to decode response as object before checking for error.

Related issue: #313

Approach

Always check if an error can be decoded first.

TODOs before merging

parse-github-assistant[bot] commented 2 years ago

Thanks for opening this pull request!

cbaker6 commented 2 years ago

@Climbatize @vdkdamian can you check if this branch solves the problem?

Also check if all other functionality works as normal. I'll take a further look later...

codecov[bot] commented 2 years ago

Codecov Report

Merging #332 (afbd466) into main (96232f3) will decrease coverage by 0.08%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #332      +/-   ##
==========================================
- Coverage   85.11%   85.03%   -0.09%     
==========================================
  Files         114      114              
  Lines       12116    12118       +2     
==========================================
- Hits        10312    10304       -8     
- Misses       1804     1814      +10     
Impacted Files Coverage Δ
Sources/ParseSwift/Extensions/URLSession.swift 72.72% <100.00%> (+0.26%) :arrow_up:
...urces/ParseSwift/API/API+NonParseBodyCommand.swift 74.52% <0.00%> (-1.28%) :arrow_down:
Sources/ParseSwift/Types/ParseAnalytics.swift 97.45% <0.00%> (-1.28%) :arrow_down:
Sources/ParseSwift/API/API+Command.swift 84.23% <0.00%> (-0.42%) :arrow_down:
Sources/ParseSwift/Objects/ParseUser.swift 82.25% <0.00%> (-0.37%) :arrow_down:

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 96232f3...afbd466. Read the comment docs.

vdkdamian commented 2 years ago

@Climbatize @vdkdamian can you check if this branch solves the problem?

Also check if all other functionality works as normal. I'll take a further look later...

I'll adopt to this branch now, and let you know in the next few days if any of the testers experienced issues.

cbaker6 commented 2 years ago

Tested this on all of the playground examples and it works along with not disturbing other code. The change makes sense logically and should be fine to add as ParseObject's don't look like ParseError's.

vdkdamian commented 2 years ago

@cbaker6 I currently haven't experienced any issues with the change either. LGTM

cbaker6 commented 2 years ago

@vdkdamian thanks for checking!

MergeMaster commented 1 year ago

I am still getting this error using ParseSwift 4.12.2 and ParseServer 5.4.0. In the server preventLoginWithUnverifiedEmail and verifyUserEmails are set to true. The error that I am getting is the one of #313.

Should I create a new issue? @cbaker6