fix(pluto): improve onlyOne() error message with diagnostic context#651
fix(pluto): improve onlyOne() error message with diagnostic context#651A-Chronicle wants to merge 3 commits into
Conversation
The onlyOne() method in Pluto threw a generic Error("something wrong") with no context about what was being looked up or why it failed.
Changes:
- Accept optional context string to identify the caller and query
- Distinguish between empty results (arr.length === 0) and null item
- Report the actual count when multiple results found
- Pass context from getPairByDID() and getPairByName() callers
Fixes hyperledger-identus#648
Signed-off-by: A-Chronicle <chaubeyanshika319@gmail.com>
A-Chronicle
force-pushed
the
fix/pluto-onlyone-error-message
branch
from
5dedf82 to
de22912
Compare
abhigyan1102
left a comment
abhigyan1102
left a comment
There was a problem hiding this comment.
Nice fix! The split between empty vs. multiple results with the actual count is way better than the old "something wrong" message.
One thing I noticed — getAllMediators() at L743 and L751 has the same problem but worse: throw new Error() with completely empty messages. Might be worth adding context there too, either in this PR or a follow-up.
Thankyou @abhigyan1102 for the review , I will surely look into the issues you have mentioned ,looking forward to fix them soon maybe in a follow up pr probably. |
getAllMediators() had bare throw new Error() calls with no message at two failure points, making debugging impossible. Changes: - Missing link error now reports the hostId and which link is missing - Empty DID error reports hostId and which DIDs resolved to null Signed-off-by: A-Chronicle <chaubeyanshika319@gmail.com>
|
@abhigyan1102 @elribonazo @yshyn-iohk I have addressed addressed the getAllMediators() issue in this updated PR, now it is ready to get reviewed. |
…) error context - Add JSDoc comment to onlyOne() explaining context parameter and failure modes - Add inline comments to getAllMediators() error throws explaining each check - Add tests verifying onlyOne() throws descriptive errors for empty/multiple results - Add tests verifying getAllMediators() throws descriptive errors for missing links/DIDs Signed-off-by: A-Chronicle <chaubeyanshika319@gmail.com>
|
|
Looks good! the |
2 participants
Description
The
onlyOne()method inPluto.tsthrows a genericnew Error("something wrong")with no diagnostic context, making it impossible to identify what query failed or why.Changes:
onlyOne()now accepts an optional context string to identify the callerarr.length === 0) and multiple resultsgetPairByDID()andgetPairByName()callersgetAllMediators()error messages (was barethrow new Error()with no message)onlyOne()andgetAllMediators()Fixes #648
Alternatives Considered
PlutoErrorsubclass — overkill for a private helper; callers already handle errors appropriatelyonlyOne()public — unnecessary; only used internallyChecklist