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

IOS Fix - NSCocoaErrorDomain error 256 #255

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

EduFrazao
Copy link

Fixed a IOS Bug, throwing NSCocoaErrorDomain code 256 whein trying to read absolutePath's of files ready to be uploaded.

@edeckers
Copy link
Owner

Thank you @EduFrazao ! I'll try to take a look and merge this weekend

@EduFrazao
Copy link
Author

Thank you @EduFrazao ! I'll try to take a look and merge this weekend
Thank you @edeckers !!!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

github-actions bot commented Apr 3, 2024

TypeScript Test Report

  1 files  ±0    1 suites  ±0   1s ⏱️ ±0s
35 tests ±0  35 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 93778d7. ± Comparison against base commit 7be63df.

Copy link
Contributor

github-actions bot commented Apr 3, 2024

Android Unit Test Report

  1 files  ±0    1 suites  ±0   9s ⏱️ ±0s
19 tests ±0  19 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 93778d7. ± Comparison against base commit 7be63df.

Copy link
Contributor

github-actions bot commented Apr 3, 2024

Android Instrumented Test Report

1 files  ±0  1 suites  ±0   1h 21m 26s ⏱️ +41s
8 tests ±0  8 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 93778d7. ± Comparison against base commit 7be63df.

Copy link
Contributor

github-actions bot commented Apr 3, 2024

iOS Test Report

  1 files  ±0    1 suites  ±0   4s ⏱️ -4s
15 tests ±0  12 ✔️  - 3  0 💤 ±0  3 ❌ +3 
16 runs  +1  12 ✔️  - 3  0 💤 ±0  4 ❌ +4 

For more details on these failures, see this check.

Results for commit 93778d7. ± Comparison against base commit 7be63df.

@edeckers
Copy link
Owner

edeckers commented Apr 7, 2024

This is taking longer than expected, as I've had some issues with the builder machine not being able to kill the test runs correctly. Also, some iOS-tests are failing and I didn't have time to investigate yet. I hope to find some time for this soon

@EduFrazao
Copy link
Author

This is taking longer than expected, as I've had some issues with the builder machine not being able to kill the test runs correctly. Also, some iOS-tests are failing and I didn't have time to investigate yet. I hope to find some time for this soon

Hi @edeckers ! Thanks for your answer!
No problem, I'm running a patched version on my application waiting for your review. Thank you very mutch.

@edeckers
Copy link
Owner

tl; dr: found the problem that causes the tests to fail, did not have time to fix yet

--

I had some time at my disposal, so I was able to track down the issue with regards to the broken tests.

The buildRequestDataForFileUpload receives a parts parameter, which contains - among other things - the textual representation of the path to the local file to be uploaded.

When this path is prefixed with file:// schema in the input, e.g. file:///path/to/your/file.ext this causes problems, because URL(fileURLWithPath: ... replaces the file:// schema with file:/, e.g. file:/path/to/your/file.ext which results in a 'file not found' exception.

There are some instant gratification hacks to circumvent this problem, like stripping file:// from the provided url, but that's just that: a hack, which will probably open a whole other can of worms and is not something that I'd like to introduce to the codebase.

So, I will need to find some time again to fix it properly, but the available time is sparse these days so it might be a while. If you're up to it, I'd happily accept a PR that addresses the situation!

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