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

Read/Write Tags/Groups error friendly await #51

Open
pcnate opened this issue Mar 25, 2019 · 1 comment
Open

Read/Write Tags/Groups error friendly await #51

pcnate opened this issue Mar 25, 2019 · 1 comment

Comments

@pcnate
Copy link
Contributor

pcnate commented Mar 25, 2019

Read/Write Tags/Groups should be more await friendly by allowing const [ error, data ] = PLC.readTag( tag )

Current Behavior

You can currently await any of these functions but if an error happens, it throws an uncaught promise error which are pretty annoying to track down. The workaround is to write a wrapper for each function that returns a promise and catch errors yourself which makes the current level of support for await pointless.

Expected Behavior

You should be able to handle all errors like so:

const [ error, data ] = PLC.readTag( tag )
if ( error ) {
    // handle the error, log error, break you current block, etc
}

Another option could be to pass an error callback but... or an options object to control return structure

Possible Solution (Optional)

Each of these functions should contain a then and a catch and both should return something and not cause unhandled promise rejection errors. Returning [ false, data ] could be a breaking change however so a callback could be added to be executed on the catch block.

I currently use await-to-js to "awaitify" these functions.

Context

Trying to use async await much more than I used to and encountering unexpected issues or forgetting to use helper functions.

Steps to Reproduce (for bugs only)

  1. Use any await example from the README
  2. Try it on a tag that doesn't exist or throws some other error
  3. Program dies

Your Environment

  • Package version (Use npm list - e.g. 1.0.6):
  • Node Version (Use node --version - e.g. 9.8.0):
  • Operating System and version:
  • Controller Type (eg 1756-L83E/B):
  • Controller Firmware (eg 30.11):
@pcnate
Copy link
Contributor Author

pcnate commented Mar 25, 2019

this might be worth implementing in v2.0

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