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

emit error event instead of throw error #5

Closed
wants to merge 1 commit into from

Conversation

fengmk2
Copy link

@fengmk2 fengmk2 commented May 21, 2013

I think just emit error event is better than throw it on ConnectionManager.prototype.onSocketData().

When network come wrong, the zk response data will fault. And node-zookeeper-client throw error when data format incorrect, it will let node process exit.

I think use error event is better. If use care about error, he will listen the error event and reconnect zk server again.

@see alibaba/node-hbase-client#22

@alexguan
Copy link
Owner

Right now, zookeeper client does not emit error event directly to the user since just throwing out errors does no good for the user.

The correct way to handle those errors are actually described in the comments. The client should disconnect and reconnect again when this kind of error happens.

Have you encountered those errors during usage?

@fengmk2
Copy link
Author

fengmk2 commented May 22, 2013

@alexguan I meet the error on my test environment. The test environment network will interrupt random, to mock network instability condition. So I can test my hbase client on the test environment and find out the potential bugs.

Because zookeeper client is async, I can't try catch the throw errors.

If zk client emit the error, I can listen the error event, and log it, then let zk client disconnect and reconnect again.

@fengmk2
Copy link
Author

fengmk2 commented May 22, 2013

If user not listen the error event, node process will just throw it.

@see http://nodejs.org/docs/latest/api/events.html#events_class_events_eventemitter

When an EventEmitter instance experiences an error, the typical action is to emit an 'error' event. Error events are treated as a special case in node. If there is no listener for it, then the default action is to print a stack trace and exit the program.

@alexguan
Copy link
Owner

I understand the problem, however, I do think there is a better way to handle it than emitting error. I'm thinking of disconnecting and reconnecting automatically when those scenarios happen.

@fengmk2
Copy link
Author

fengmk2 commented May 23, 2013

I see. I'm waiting for your implementation. I want to use zk client on my production environment as soon as posible.

@alexguan
Copy link
Owner

working on it now, will post an update soon

@fengmk2
Copy link
Author

fengmk2 commented May 24, 2013

Cool!
在 2013-5-24 AM9:33,"Alex Guan" [email protected]写道:

working on it now, will post an update soon


Reply to this email directly or view it on GitHubhttps://github.com//pull/5#issuecomment-18382012
.

@fengmk2
Copy link
Author

fengmk2 commented May 30, 2013

Any progress?

@alexguan
Copy link
Owner

Got stuck with work, will address this soon, hopefully before the weekend.

alexguan added a commit that referenced this pull request Jun 3, 2013
alexguan added a commit that referenced this pull request Jun 3, 2013
@fengmk2 fengmk2 closed this Jun 4, 2013
@fengmk2 fengmk2 deleted the emit-error branch June 4, 2013 01:47
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