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

feat: Add invalid struct error #238

Merged
merged 31 commits into from
Oct 9, 2021
Merged

Conversation

dblythy
Copy link
Member

@dblythy dblythy commented Sep 20, 2021

New Pull Request Checklist

Issue Description

Invalid structs are really hard to isolate the cause of the problem, especially with larger classes

Related issue: #237

Approach

Checks if error code is 4865 and returns error .invalidStruct if so

TODOs before merging

  • Add tests
  • Add entry to changelog

@parse-github-assistant
Copy link

parse-github-assistant bot commented Sep 20, 2021

Thanks for opening this pull request!

  • 🎉 We are excited about your hands-on contribution!

@dblythy dblythy changed the title Add invalid struct error feat: Add invalid struct error Sep 20, 2021
@codecov
Copy link

codecov bot commented Sep 20, 2021

Codecov Report

Merging #238 (f9b27f8) into main (7aeef06) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #238      +/-   ##
==========================================
- Coverage   76.72%   76.70%   -0.02%     
==========================================
  Files          99       99              
  Lines       10374    10382       +8     
==========================================
+ Hits         7959     7964       +5     
- Misses       2415     2418       +3     
Impacted Files Coverage Δ
Sources/ParseSwift/API/URLSession+extensions.swift 73.12% <100.00%> (+0.86%) ⬆️
Sources/ParseSwift/Types/ParseError.swift 100.00% <100.00%> (ø)
Sources/ParseSwift/Coding/ParseEncoder.swift 73.25% <0.00%> (-0.36%) ⬇️

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 7aeef06...f9b27f8. Read the comment docs.

Sources/ParseSwift/API/URLSession+extensions.swift Outdated Show resolved Hide resolved
Sources/ParseSwift/Types/ParseError.swift Outdated Show resolved Hide resolved
Sources/ParseSwift/Types/QueryViewModel.swift Outdated Show resolved Hide resolved
Sources/ParseSwift/Types/QueryViewModel.swift Outdated Show resolved Hide resolved
Sources/ParseSwift/Types/CloudViewModel.swift Outdated Show resolved Hide resolved
Sources/ParseSwift/Types/CloudViewModel.swift Outdated Show resolved Hide resolved
@cbaker6
Copy link
Contributor

cbaker6 commented Sep 20, 2021

It also looks like you broke some Linux tests as well. Those will need to be fixed before the PR can be approved.

@dblythy
Copy link
Member Author

dblythy commented Sep 20, 2021

The test that is failing on Linux is the one that I added (the different response for the .invalidStruct error). Does linux handle swift json decoding differently?

@cbaker6
Copy link
Contributor

cbaker6 commented Sep 20, 2021

The test that is failing on Linux is the one that I added (the different response for the .invalidStruct error). Does linux handle swift json decoding differently?

Yes, there are some differences with key order and it's possible linux throws a different NSError as the error code you are using might come from obj-c. If you want to see the exact error code, you probably will need to add a print statement and then check the linux CI for the value of the code

@cbaker6
Copy link
Contributor

cbaker6 commented Sep 22, 2021

Let me know if you need any help with finishing this. After this PR is merged, I'll release a new version.

@dblythy
Copy link
Member Author

dblythy commented Sep 22, 2021

Sorry I’ll try wrap this up today!

@cbaker6 cbaker6 linked an issue Sep 23, 2021 that may be closed by this pull request
3 tasks
@vdkdamian
Copy link
Contributor

@dblythy Will this also show better error logging for when a certain key that is required in the database isn't filled in?

@dblythy dblythy marked this pull request as draft October 8, 2021 02:09
@dblythy
Copy link
Member Author

dblythy commented Oct 8, 2021

Still testing locally to make sure this doesn't clash with other error codes

@dblythy
Copy link
Member Author

dblythy commented Oct 9, 2021

@novemTeam I'm not sure to be honest, what error do you currently get when that happens?

@dblythy dblythy marked this pull request as ready for review October 9, 2021 06:49
@cbaker6 cbaker6 merged commit 85366e6 into parse-community:main Oct 9, 2021
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.

Better error logging for Invalid Struct
3 participants