-
Notifications
You must be signed in to change notification settings - Fork 475
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
Order of operations on error/close with connections/channels #20
Comments
Channels are closed first in most (if not all) other clients. |
Some clients (noteably the Java one and all the others that build on top of it) provide you additional information |
So if a channel has an error and I want to reconnect it, can I differentiate between when the channel is closed because of the connection being closed? |
Actually I think that statement in the docs is wrong. I'll fix that.
The idea is channels close first, then the connection. I do wonder what is most useful for applications though:
Maybe it's better to only emit 'close' if just that channel has closed (as it is now).
Well, at the minute, you'll only see the connection close. But were I to change that, I guess there'd have to be some indication in the event, since you'd see the channels close before the connection. It's pretty ugly, isn't it. |
The current behavior was actually what I was hoping for. I could have tested it out, but I wanted to know if it was actually defined and reliable, if that makes sense. I imagine any reasonably organized program is going to have channels as subsets somehow of the connection, so I don't see a great need for issuing events for both. Additionally, it makes retry/reconnect behavior simpler if you can rely on the fact that a channel close message means the connection is still open. Alternatively, if you emit the connection event before the channels, guaranteed, then the programmer can do what they want accurately -- if the channel emits an error, they can set a property or something to indicate the channel is defunct, or manually clean up the channels such that the events won't have any effect, etc. |
Tangentially related: is there a list of potential errors somewhere? I'd like to distinguish between AMQP errors and, say, stupid typos in my source code ;) Edit: nevermind, forgot about defs.js |
See Constants. |
I can't tell if there's a way to access these values from any error handler, but I was thinking more along the lines of a general way to identify errors that are sourced from amqp or the amqp connection vs other errors. Since promises get used, a thrown error in 'my' code gets caught in the library for example, and if I screwed up I want it to crash or act accordingly. On the other hand, an error that causes a channel closure I want to attempt to recover from. I'm not sure how to distinguish the two. I can either check for TypeError, ReferenceError, and whatever else may exist explicitly, or possibly create an error subclass for amqp. I forked the repo, think I'll do the latter. |
@myndzi In general the variety of error emitted isn't very helpful, except insofar as an error invalidates specifically the channel or the connection. Errors in application code (e.g., a consumer callback that throws an exception) should only ever invalidate the channel; connection errors are always protocol breaches or server problems. That's a good point about promises: at the minute you may not be able to distinguish easily between an RPC failure (trying to check a missing queue) and an application code failure (trying to use an undeclared variable). In only the first case the channel will emit 'error', but it may not have done that by the time the promise is rejected. Ugh. I'll have to think about this. |
Argh no I am wrong, and the docs are accurate: https://github.com/squaremo/amqp.node/blob/v0.0.2/lib/connection.js#L273 So, channels are implicitly closed when the connection stops (i.e., when it is itself closed). But they won't emit an error. In summary:
( NB in the case of promises returned from The tricky one I can see there is know whether a channel has rejected your RPC because e.g., the queue you mentioned doesn't exist, or because the server decided to close the connection: at the point the RPC is rejected you may not have seen the connection error event. Given the difficulty of making things happen in a defined order in Node.JS, the way to distinguish those may be to have different types of error (used both for rejecting promises and for emitting). Usually I don't like dispatching by exception types. Maybe the thing to do is just to do a crash stop -- if the connection's closed, you're hosed anyway. In that scenario the channels would just be left alone (no RPC rejections, no events emitted). |
.. by which I mean, emit an |
It occurs to me that if the connection issued 'error' immediately on receiving ConnectionClose, that would make all the cases distinguishable. |
Personally, I'd rather have an explicit distinction (Error type, or a property set on the error object, etc.) than an implicit distinction (Connection error emitted first). As you note above, making things happen in a defined order is tricky -- and for that same reason, expecting things to happen in a defined order is not intuitive. |
@myndzi Yes, fair point. What about the 'crash stop' option? |
I think I realized a good solution. Include the connection in the channel close event. The user can use that object to reestablish the channel, and if the connection is closed, that will result in an error. This is distinct from doing something like:
Because in that case, it's possible(?) for 'self.conn' to be reassigned to the new connection before it's referenced in the event. If you supply the connection that the channel was bound to, there is no ambiguity... (I suppose the channel object must have a connection property somewhere, I could use that to fulfill my need, but making it explicit in this way makes it a lot more obvious) |
This seems like a solution that's particular to your code. There is precedent for including a flag in the close event, but even that I am wary of, since it duplicates information provided by emitting Here's the situation on master:
For the most part the 'close' events aren't that useful, since they always happen; you might want to log something, perhaps. More interesting are the error continuations (rejected promises, or callbacks invoked with errors if you use the 'raw' channel interface) and 'error' events. In general you'd would use the error continuations for channel operations, and the error event for connections. In theory you'll know by the time error continuations are invoked whether the connection has been closed, since that always fires first. So, if you care why an operation failed, you can check against some flag you set Thinking about it though, there's no reason the client library oughtn't help out by encoding that hypothetical flag in the errors. Using different error constructors seems like a sound way to do it. |
The docs say that channels implicitly emit close events when a connection closes, but I can't see any code in channel_model.js or channel.js that does this (I assume it's precipitated lower down?)
Either way, say you have a connection with two channels. Is there a defined order that the events will trigger in? If it errors, will I receive three close events and three error events? If it closes, will I receive three close events?
Will the connection emit its event first, or the channels?
Is there a way to tell in the channel close/error event if it was caused by the connection closing? Is there a property to determine the current state of a connection/channel in general?
Sorry if any of these are answered in the docs, I did look first.
The text was updated successfully, but these errors were encountered: