silexlabs / unifile

Unified access to cloud storage services through a simple web API.
https://silexlabs.github.io/unifile/
MIT License
144 stars 32 forks source link

Error codes #103

Open lexoyo opened 7 years ago

lexoyo commented 7 years ago

Following our discussion: unifile should return error status (not logged in, 404, server error...) + the text which is currently returned

If you give me instructions on how to do this I probably will

JbIPS commented 7 years ago

What I had in mind:

What do you say?

lexoyo commented 7 years ago

Great! It would cover my use case I'll see if I can do this soon

lexoyo commented 7 years ago

If we follow your philosophy we should also mimic nodejs API for errors https://nodejs.org/api/errors.html#errors_common_system_errors

lexoyo commented 7 years ago

With fs, when one try to delete a non-empty folder it throws this error: "errno":-39,"code":"ENOTEMPTY","syscall":"rmdir","path":"/home/lexoyo/tmp/publish"

Should dropbox service do the same?

lexoyo commented 7 years ago

Or maybe fs should remove the folder recursively

JbIPS commented 7 years ago

That's a difficult question since I'm mot very fond of fs API, but it's based on Unix so we should probably follow it.

On the other hand, I think connectors should implement a recursiveDelete()-ish function.

lexoyo commented 6 years ago

Can you confirm @JbIPS that is missing for most connectors (all but github)

JbIPS commented 6 years ago

I'm not sure of what you're saying but

recursive delete like fs does

fs doesn't do that, github do and I think dropbox too (to be tested)

use UnifileError and BatchError to throw errors (e.g. "path not found")

I'm implementing that in dropbox right now

lexoyo commented 6 years ago

fs doesn't do that

My bad Then I think we should not stick to fs for this behavior and make it recursive for every service, what do you think? Or add the recursiveDelete method as you suggested?

JbIPS commented 6 years ago

I figured out yesterday it would be very difficult to constrain GitHub and Dropbox to not remove a non-empty folder. So, for coherence sake, maybe Fs should do it too.

Maybe we should have a 'compatibility mode' (default to false) in fs constructor that would enforce that. When false the rm would remove an entire folder empty or not.

Would that be OK with everyone?