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

Error handling api design doesn't allow determining whetger getAttribute operation failed #19

Closed
kdvolder opened this issue Aug 3, 2018 · 1 comment

Comments

@kdvolder
Copy link

kdvolder commented Aug 3, 2018

Let's say we have code like this:

client.getAttribute(mbean, attrib, data => {
      ...do something with the data..
});

The way the api is designed there seems to be no way to change this code so that it can properly handle errors arising from that operation.

Yes, we can do something like this:

client.on('error', (error) => ...handle error..);

But... the error handler can not know what operation caused the error. Even if we use client.on in combination with client.of to only attach error handlers for the duration of an operation, we will still have issues to accurately determine which operation caused the error when more than one operation is executed in parallel.

So, to make story short, the api should really allow for something like this:

client.getAttribute(mbean, attrib, (data, err) => {
    if (error) {
      ..handle error...
   } else {
     ..do something with the data..
   }
});

Note: if the api was designed like this from the start, I would probably put the 'err' parameter before the 'data' parameter as that seems to be the prevailing convention for asynchronous callbacks in nodejs libraries.

But seeing as the api was not designed to work like this initially, putting the 'err' parameter second means that it will not break existing code that assumes the first parameter is the data.

@kdvolder
Copy link
Author

kdvolder commented Aug 3, 2018

Ah I discovered there is already another bug ticket that seems to cover this problem:

#8

I'll close this ticket then.

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

No branches or pull requests

1 participant