FirebaseExtended / angularfire

AngularJS bindings for Firebase
MIT License
2.73k stars 632 forks source link

Catching permission errors with $asObject().$loaded() #401

Closed mickhansen closed 10 years ago

mickhansen commented 10 years ago

I'm using GeoFire with angularFire. geoFire will give me a list of items nearby, and i'll load each one with angularFire. Now some of these items are not available to the user because of privacy reasons (implemented with security rules).

$firebase(new Firebase('[firebaseUrl]/events/'+key)).$asObject().$loaded().then(function (event) {
  // Do stuff
}).catch(function (error) {
  console.log(error);
});

In this case i just want to ignore the error. But firebase is still both logging a warning and throwing an error, how do i stop it throwing an error when i'm already catching it?

katowulf commented 10 years ago

You can't stop this; it's not a feature of AngularFire, but a behavior of the core Firebase lib. If you want to eradicate the warning, you'll need to structure your data in a manner that you can tell if it is accessible before you do a read op. In general, it would be a bug if the UI attempts to read data a user cannot access.

mickhansen commented 10 years ago

But AngularFire allows you to catch the event? Makes little sense that you can't ignore the global one if you handle it at a code level.

I'm not sure i'm able to structure the data like you propose with GeoFire.

katowulf commented 10 years ago

The console warning is not the same thing. AngularFire uses the callback from Firebase, which returns an error on failure. But that does not control/catch/coincide with the console warning. Sorry.

katowulf commented 10 years ago

You may want to raise an issue on the GeoFire repo (or the mailing list) and see if that community has some ideas for dealing with this, if you feel it is specific to the GeoFire architecture; I'm not versed there so I can't be much help.

mickhansen commented 10 years ago

I'm not talking about the warning, i'm talking about the actual error. I'm getting both the error callback and the global error thrown, i'd like not to have the global error. I'm fine with the warning.

With GeoFire you can only store a simple value and GPS coordinates, so all you get back is the simple value. So i don't quite see how i'd be able to not check the access from the frontend.

mickhansen commented 10 years ago

I got a few ideas on how to do the check first, it adds some overhead but i guess i'll have to do with it :) I still have an issue with the global error thrown though, seems off to me that you catch the error but it's still thrown.

katowulf commented 10 years ago

Oh, a double error is a bit different. You may be seeing the Angular $log.error that is printed here: arrays objects

That could easily be turned off using $log 's config settings or by simply extending $$error using $extendFactory.

mickhansen commented 10 years ago

It's being thrown from firebase.js. It seems like it's throwing it even though angularFire should be listening to it and catching it.

katowulf commented 10 years ago

Yes, permission errors are logged directly from Firebase.js. Those are warnings. $$error does log a console.error any time an event is raised. But nothing is actually "thrown" from Firebase.js for security errors--that I know of, at least. You may want to include a screenshot and code sample if you are seeing something different.

mickhansen commented 10 years ago
Error: permission_denied: Client doesn't have permission to access the desired data.
    at Error (native)
    at cc (https://cdn.firebase.com/js/client/1.0.17/firebase.js:43:333)
    at https://cdn.firebase.com/js/client/1.0.17/firebase.js:111:200
    at https://cdn.firebase.com/js/client/1.0.17/firebase.js:80:207
    at ld.h.cc (https://cdn.firebase.com/js/client/1.0.17/firebase.js:85:104)
    at $c.cc (https://cdn.firebase.com/js/client/1.0.17/firebase.js:76:364)
    at Q.Td (https://cdn.firebase.com/js/client/1.0.17/firebase.js:74:280)
    at Ic (https://cdn.firebase.com/js/client/1.0.17/firebase.js:59:234)
    at WebSocket.W.onmessage (https://cdn.firebase.com/js/client/1.0.17/firebase.js:58:111) 

Logged as an error not a warning and appears to come from firebase rather than angularFire. But overwriding $$error does appear to work. I would prefer a situation where i wouldn't have to do that when implementing catch() - I'd like to have a global error if i'm not handling it directly (since then something dumb is happening).

But $$error works, so guess i'll do with that for now.

mickhansen commented 10 years ago

In any case, thank you for your time.

katowulf commented 10 years ago

Cheers; thanks for the feedback. We'll chat about this today during our AngularFire feature discussion.