LIT-Protocol / js-sdk

The Lit Protocol SDK provides developers with a framework for implementing Lit functionality into their own applications.
https://developer.litprotocol.com
MIT License
99 stars 59 forks source link

always return the request id in the error #490

Closed glitch003 closed 3 months ago

glitch003 commented 4 months ago

Pretty simple change - just return the request id if it is actually present, even if the error is an unknown type. this makes debugging a lot easier, in cases where we aren't getting a good error back from the nodes for some reason.

joshLong145 commented 4 months ago

@glitch003 Only thing that concerns me with this is the cases where we throw errors due to validation and we have not yet sent any request to a Lit Node. If we introduce the request id validation errors client side will get this value and it wont map to a node request.

glitch003 commented 4 months ago

for sure - in that case, we just print "no request id found" as you can see here https://github.com/LIT-Protocol/js-sdk/pull/490/files#diff-f8df194ec35cd4e8d5042228b9163d51921c46776ca48fac0613bdbd87e965f9R208

so i think this is fine, because like it doesn't add any additional requirements. call throwError with or without a request id, it still works. but, if there is a request id, then at least print it.

this PR came from my frustrations of running the SDK with debug off, and occasionally getting this kind of error with no request id in it, which made it hard to debug on the node side. i did manually test this in pkp-parallel-keygen and it showed the request id, so it accomplished my goal.

joshLong145 commented 3 months ago

Ah yes we do see that. Makes sense l the traces will be more consistent. Merging now!