dgram: better error messages#250
Conversation
This will allow util._detailedException to be used in both net and dgram for better error messages.
Add address and port to errors where applicable for better reporting
|
very nice, will leave it to someone more expert on this module to "lgtm" this though |
There was a problem hiding this comment.
I think this name is too generic, createDetailedBindException, or exceptionWithHostPort would be better. Other libraries, like fs, might also need detailed exceptions, but with different properties.
There was a problem hiding this comment.
That's a good point. I would think the exceptionWithHostPort would be more descriptive. I'll go ahead and change it to that if that works.
|
I like it, net and dgram should have same detail in exceptions. |
|
@evanlucas can you give a before and after of the error message? |
|
Before: After: |
|
That's better already, but it would be nice if when printed it reads |
|
lgtm, a marked improvement. I'll land after running the tests. |
This allows _detailedException() to be used by both the 'net' and 'dgram' modules to provide more informative error messages. PR-URL: #250 Reviewed-By: Bert Belder <[email protected]>
The _detailedException() helper function used to be local to the 'net' module, but now that it has been moved to 'util' a more descriptive name is desirable. PR-URL: #250 Reviewed-By: Bert Belder <[email protected]>
PR-URL: #250 Reviewed-By: Bert Belder <[email protected]>
|
Thanks, landed in 8a0e7d6...c9fd9e2 |
Port of the io.js util._exceptionWithHostPort addition. Slight change to omit the ex.address and ex.port from the generated error to avoid the API change. These are commented out for now so they can be added back in easily later. Pulled over the changes made to dgram and net to use this. Added an additional test-net-server-errmsg test as additional verification see: nodejs/node#250 for reference. Addresses: nodejs#9294 /cc @joyent/node-coreteam
Port of the io.js util._exceptionWithHostPort addition. Slight change to omit the ex.address and ex.port from the generated error to avoid the API change. These are commented out for now so they can be added back in easily later. Pulled over the changes made to dgram and net to use this. Added an additional test-net-server-errmsg test as additional verification see: nodejs/node#250 for reference. Addresses: nodejs#9294 /cc @joyent/node-coreteam
Port of the io.js util._exceptionWithHostPort addition. Slight change to omit the ex.address and ex.port from the generated error to avoid the API change. These are commented out for now so they can be added back in easily later. Pulled over the changes made to dgram and net to use this. Added an additional test-net-server-errmsg test as additional verification see: nodejs/node#250 for reference. Addresses: nodejs#9294 /cc @joyent/node-coreteam
5 participants
This PR adds
addressandportproperties to dgram error messages for better reporting.