| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
I also found that falsy causes are not inspected, but that can be fixed in a separate PR. |
Sorry, something went wrong.
|
@targos how would you handle undefined and null? I would at least skip those? |
Sorry, something went wrong.
|
I would always inspect if Object.hasOwn(err, 'cause') === true with no exception. |
Sorry, something went wrong.
|
@targos I added another commit to also inspect falsy values besides undefined. Undefined is used in argument default values as "does not exist" and I would handle it here identically. |
Sorry, something went wrong.
An error cause may be of any type. Handle all of them, no matter if they are an error or not. Fixes: nodejs#41096 Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
Okay, I don't have a strong opinion, but new Error('message', { cause: undefined }) creates the property while just new Error('message') doesn't. |
Sorry, something went wrong.
Sorry, something went wrong.
|
@nodejs/util PTAL. This should ideally land before the next release. |
Sorry, something went wrong.
Commit Queue failed- Loading data for nodejs/node/pull/41097 ✔ Done loading data for nodejs/node/pull/41097 ----------------------------------- PR info ------------------------------------ Title util: make sure error causes of any type may be inspected (#41097) Author Ruben Bridgewater (@BridgeAR) Branch BridgeAR:fix-non-error-cause-inspection -> nodejs:master Labels util, author ready, needs-ci Commits 2 - util: make sure error causes of any type may be inspected - util: serialize falsy cause values while inspecting errors Committers 1 - Ruben Bridgewater PR-URL: https://github.com/nodejs/node/pull/41097 Fixes: https://github.com/nodejs/node/issues/41096 Reviewed-By: Michaël Zasso Reviewed-By: James M Snell ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/41097 Fixes: https://github.com/nodejs/node/issues/41096 Reviewed-By: Michaël Zasso Reviewed-By: James M Snell -------------------------------------------------------------------------------- ℹ This PR was created on Mon, 06 Dec 2021 13:34:46 GMT ✔ Approvals: 2 ✔ - Michaël Zasso (@targos) (TSC): https://github.com/nodejs/node/pull/41097#pullrequestreview-824046654 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/41097#pullrequestreview-826822688 ✔ Last GitHub Actions successful ℹ Last Full PR CI on 2021-12-07T19:10:46Z: https://ci.nodejs.org/job/node-test-pull-request/41399/ - Querying data for job/node-test-pull-request/41399/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/master up to date... From https://github.com/nodejs/node * branch master -> FETCH_HEAD ✔ origin/master is now up-to-date - Downloading patch for 41097 From https://github.com/nodejs/node * branch refs/pull/41097/merge -> FETCH_HEAD ✔ Fetched commits as 18ff5832501b..38fc7f86a0bf -------------------------------------------------------------------------------- Auto-merging lib/internal/util/inspect.js [master 12b79bd4f5] util: make sure error causes of any type may be inspected Author: Ruben Bridgewater Date: Mon Dec 6 14:22:30 2021 +0100 3 files changed, 39 insertions(+), 1 deletion(-) Auto-merging lib/internal/util/inspect.js Auto-merging test/parallel/test-util-inspect.js [master 1272d9fe5a] util: serialize falsy cause values while inspecting errors Author: Ruben Bridgewater Date: Mon Dec 6 15:25:42 2021 +0100 2 files changed, 12 insertions(+), 1 deletion(-) ✔ Patches applied There are 2 commits in the PR. Attempting autorebase. Rebasing (2/4)https://github.com/nodejs/node/actions/runs/1555617774 |
Sorry, something went wrong.
|
@BridgeAR this didn't land cleanly into the release, can you backport this to v17.x-staging? |
Sorry, something went wrong.
|
@danielleadams this should land cleanly on top of #41002. |
Sorry, something went wrong.
An error cause may be of any type. Handle all of them, no matter if they are an error or not. Fixes: #41096 Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de> PR-URL: #41097 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
|
@BridgeAR the spec for error cause very intentionally differentiates between an absent property, and a present undefined, so it's critically important that util.inspect differentiate between these two. |
Sorry, something went wrong.
|
@ljharb I am fine if you like to also visualize an undefined cause. Please feel free to open another PR. |
Sorry, something went wrong.
See #41097 (comment) PR-URL: #41247 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com>
See #41097 (comment) PR-URL: #41247 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com>
An error cause may be of any type. Handle all of them, no matter if they are an error or not. Fixes: #41096 Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de> PR-URL: #41097 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
An error cause may be of any type. Handle all of them, no matter if they are an error or not. Fixes: #41096 Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de> PR-URL: #41097 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
See #41097 (comment) PR-URL: #41247 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com>
An error cause may be of any type. Handle all of them, no matter if they are an error or not. Fixes: #41096 Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de> PR-URL: #41097 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
See #41097 (comment) PR-URL: #41247 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com>
| Back | FazBrowse Home | New Git URL |
An error cause may be of any type. Handle all of them, no matter
if they are an error or not.
Fixes: #41096
Signed-off-by: Ruben Bridgewater ruben@bridgewater.de