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.

Sign up for GitHub

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

Jump to bottom

Callback function should receive error parameter #2

Closed
abmusse opened this issue Feb 27, 2017 · 5 comments
Closed

Callback function should receive error parameter #2

abmusse opened this issue Feb 27, 2017 · 5 comments
Labels
enhancement New feature or request
Milestone
Version 1.0

Comments

@abmusse
Copy link
Member

abmusse commented Feb 27, 2017

Original report by Aaron Bartell (Bitbucket: aaronbartell, GitHub: aaronbartell).

Currently the iToolkit callback function only operates on a single result parameter and doesn't also include an error parameter. Errors could result from the DB2 connection.

Would be good to add this so errors can be conveyed.
@abmusse
Copy link
Member Author

abmusse commented Mar 11, 2017

Original comment by Xu Meng (Bitbucket: mengxumx, GitHub: dmabupt).

Yes, this is a design issue. We should follow the Node.js callback style -- https://nodejs.org/dist/latest-v4.x/docs/api/errors.html#errors_node_js_style_callbacks
But the new error parameter is the first parameter of functions, which may break current user code. Maybe we'd better make the changes in next majoy version upgrade.

@abmusse abmusse added minor enhancement New feature or request labels Jan 22, 2019
@kadler kadler added this to the Version 1.0 milestone Jan 22, 2019
@kadler kadler removed the minor label Jan 24, 2019
@kadler kadler changed the title Error conveyance Callback function should receive error parameter Jan 24, 2019
@abmusse abmusse mentioned this issue Jan 25, 2019
Closed
abmusse referenced this issue in abmusse/nodejs-itoolkit Mar 5, 2019
@abmusse
Rework Connection.js see issues IBM#25 and #2
f6365ba 
- allow Connection.js to accept a single object for construction IBM#25
- rework Connection.run() function to return Errors #2
- rename internal variables away from I_* prefix
@abmusse abmusse mentioned this issue Mar 5, 2019
Closed
abmusse referenced this issue in abmusse/nodejs-itoolkit Mar 7, 2019
@abmusse
Rework Connection.js see issues IBM#25 and #2
8b545d9 
- allow Connection.js to accept a single object for construction IBM#25
- rework Connection.run() function to return Errors #2
- rename internal variables away from I_* prefix
abmusse referenced this issue in abmusse/nodejs-itoolkit Mar 7, 2019
@abmusse
Rework Connection.js see issues IBM#25 and #2
9d9455b 
- allow Connection.js to accept a single object for construction IBM#25
- rework Connection.run() function to return Errors #2
- rename internal variables away from I_* prefix
- rename conn -> transportOptions
- rename getConnection() -> getTransportOptions()
- leave setting defaults and validation of transportOptions up to the
transport
- rename cmd -> commandList
@abmusse abmusse mentioned this issue Mar 7, 2019
Merged
abmusse referenced this issue in abmusse/nodejs-itoolkit Mar 8, 2019
@abmusse
Rework Connection.js see issues IBM#25 and #2
a94f0e9 
- allow Connection.js to accept a single object for construction IBM#25
- rework Connection.run() function to return Errors #2
- rename internal variables away from I_* prefix
- rename conn -> transportOptions
- rename getConnection() -> getTransportOptions()
- leave setting defaults and validation of transportOptions up to the
transport
- rename cmd -> commandList
abmusse referenced this issue in abmusse/nodejs-itoolkit Mar 11, 2019
@abmusse
Rework Connection.js see issues IBM#25 and #2
6eb3a2d 
Allow Connection.js Constuctor to accept a single object see IBM#25
Allow Connection.run() function to return error as first param of callback see #2

Establish one code path in Connection Constructor
- when pre v1.0 signature is used create an options object
- add properties to this options object
- this mimics the current v1.0 way of passing a signle object
- that way constructor follows on code path

Options object contain sub object transportOptions
- This contains properties such as databse, username, etc
- This sub object is passed to the transport

Rename variables away from I_* prefix
Rename getConnection() -> getTransportOptions()
Rename cmd -> commandList

Moved verbose to be a property of Connection
- verbose is added to transportOptions during run()
- this is because verbose can be adjusted with debug()
- add print of xmlOutput also when in verbose mode
abmusse added a commit that referenced this issue Mar 11, 2019
@abmusse
Rework Connection.js see issues #25 and #2
4dcb183 
Allow Connection.js Constuctor to accept a single object see #25
Allow Connection.run() function to return error as first param of callback see #2

Establish one code path in Connection Constructor
- when pre v1.0 signature is used create an options object
- add properties to this options object
- this mimics the current v1.0 way of passing a signle object
- that way constructor follows on code path

Options object contain sub object transportOptions
- This contains properties such as databse, username, etc
- This sub object is passed to the transport

Rename variables away from I_* prefix
Rename getConnection() -> getTransportOptions()
Rename cmd -> commandList

Moved verbose to be a property of Connection
- verbose is added to transportOptions during run()
- this is because verbose can be adjusted with debug()
- add print of xmlOutput also when in verbose mode
@abmusse
Copy link
Member Author

abmusse commented Mar 13, 2019

PR #42 allows Connection.run() to invoke users callback with error first and xml output second,

This can be configured by setting returnError: false within Connection constructor.

@kadler
Copy link
Member

kadler commented Mar 13, 2019

@abmusse To be clear, the default is to return errors (returnError: true) when using the new "single object" constructor, but to not return errors when using the old style with multiple parameters, correct?

@abmusse
Copy link
Member Author

abmusse commented Mar 13, 2019

@kadler Correct when using the old style (db, username, password, [options]) returnError is set to false.

@abmusse
Copy link
Member Author

abmusse commented Apr 17, 2019

Closing v1.0.0 will include error first callback within connection.run() by default when using new "single object" constructor.

@abmusse abmusse closed this as completed Apr 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
Version 1.0
Development

No branches or pull requests
2 participants
@kadler @abmusse