pubnub / haskell

PubNub Haskell SDK
Other
19 stars 16 forks source link

StatusCodeException #5

Closed copumpkin closed 10 years ago

copumpkin commented 10 years ago

My daemon was happily chugging along when I got a:

*** Exception: StatusCodeException (Status {statusCode = 403, statusMessage = "Forbidden"}) [("Date","Tue, 14 Jan 2014 11:10:00 GMT"),("Content-Type","text/javascript; charset=UTF-8"),("Transfer-Encoding","chunked"),("Connection","keep-alive"),("Cache-Control","no-cache, no-store, must-revalidate"),("Access-Control-Allow-Origin","*"),("Access-Control-Allow-Methods","GET"),("X-Response-Body-Start","{\"status\":403,\"service\":\"Access Manager\",\"error\":true,\"message\":\"Forbidden\",\"payload\":{\"channels\":[\"049f65dc-3af3-4ffd-85a5-aac102b2a579\"]}}\n")] (CJ {expose = []})

that killed it. I asked in #pubnub and it's apparently a status code the library should be handling.

stephenlb commented 10 years ago

SDK will always "Retry" forever until user calls unsubscribe().

subscribe() is truth for all time. the only thing which can undo a subscribe is an unsubscribe().

tsloughter commented 10 years ago

Thanks. And yup, easy enough, I'll add handling for that to retry and/or send it to onError callback.

tsloughter commented 10 years ago

Oh wait, was this on connect?

copumpkin commented 10 years ago

I had been connected for ages (hours or days) and at some point it gave me that and bombed out. I'm assuming one of the many behind-the-scenes HTTP requests got the 403.

tsloughter commented 10 years ago

Ok, yea, I realized after sending that that connect is also used to reconnect :). So probably still connect where it is erroring, since subscribe' always retries for an exception.

So I'll have to add that to connect/reconnect.

geremyCohen commented 10 years ago

@tsloughter the general pattern we're using in the other clients for PAM access denied handling is to send all 401 and 403 responses to the error callback.

From here, the developer is free to do whatever he wants based on the scenario at hand. (Insert a sleep, retry if foo, unsubscribe if bar, etc).

If you could implement that pattern, it would be excellent! :)

geremy

stephenlb commented 10 years ago

Geremy,

There is a know reason we can chat about that will block the stream. Always retry. There is a specific reason that I need to chat with you about at the office. Basically we need to retry always unless the user unsubscribes(). :smile:

geremyCohen commented 10 years ago

@stephenlb Ok, we'll chat in person about it.

stephenlb commented 10 years ago

A, correction: always retry on all errors, also fire error callbacks as Geremy describes and continue to retry forever until the user executes the unsubscribe() method.

Basically you are subscribed forever and the SDK continues to retry until the end of time or the user specifically unsubscribes().

Geremy correct me if I'm wrong but that's pretty much the jist of it?

stephenlb commented 10 years ago

Geremy, If my last comment is true, then my concern is of no consequence. :+1: We can still chat back at the office about specifics.

stephenlb commented 10 years ago

subscribe() is truth for all time. the only thing which can undo a subscribe is an unsubscribe().

copumpkin commented 10 years ago

Yeah, from a user perspective, the 403 means nothing to me, so I'd prefer not to have to handle it. If an error gives me a meaningful decision to have to make, then I should be able to make it, but it doesn't sound like this one does.

stephenlb commented 10 years ago

correcto. :)

tsloughter commented 10 years ago

Cool, I'll handle this over the weekend.

stephenlb commented 10 years ago

:+1: :smile:

tsloughter commented 10 years ago

I pushed a fix. But it still has some holes I haven't had time to deal with yet.

Right now it simply reconnects if it gets an exception that isn't a status code exception. If it is a status code exception it sends that to the error callback and then reconnects.

One question, should it simply continue on like there was no error (after calling the error callback) or reconnect? I wasn't sure if it could mean some bad state somewhere so better to just reconnect.

And is the disconnect callback only called on unsubscribe? Or should that be fired for some exceptions?

tsloughter commented 10 years ago

I'd like to get all this logic solid and correct before doing PAM, switching to the stream and looking at alternative interfaces to callbacks.

stephenlb commented 10 years ago

@tsloughter correct, continue onward as if there are no errors in all scenarios. And Ty for the patch!

tsloughter commented 10 years ago

I think it is pretty good now. @copumpkin agree? Can we close this issue?

copumpkin commented 10 years ago

Haven't had time to update my stuff to use it yet, and likely won't until the weekend. I'd just close it and I'll reopen it if it seems problematic :)