fix(nodejs): only call get_current_error when an error occurred #209
Conversation
berendsliedrecht
force-pushed
the
reduce-get-error-calls
branch
2 times, most recently
from
c71d4d2 to
5684859
Compare
Signed-off-by: Berend Sliedrecht <[email protected]>
berendsliedrecht
force-pushed
the
reduce-get-error-calls
branch
2 times, most recently
from
a413660 to
4676879
Compare
|
It's possible to keep the Android build on 1.67 by adding "--locked" to the cargo install command for cross: hyperledger/indy-vdr#247 |
berendsliedrecht
force-pushed
the
reduce-get-error-calls
branch
2 times, most recently
from
ab20d77 to
d7bd028
Compare
Signed-off-by: Berend Sliedrecht <[email protected]>
berendsliedrecht
force-pushed
the
reduce-get-error-calls
branch
from
d7bd028 to
8719c75
Compare
Maybe I am completely blind and missed something, but it does not seem to work. |
|
It seems like cross is built successfully built now, but home 0.5.9 is brought in somehow after the aries builder docker image is loaded? |
|
On the other PR I added Cargo.lock after running |
| } | ||
| const { nativeCallback, id } = toNativeCallbackWithResponse(cb, responseFfiType) | ||
| method(nativeCallback, +id) | ||
| const errorCode = method(nativeCallback, +id) | ||
| if (errorCode !== 0) deallocateCallbackBuffer(+id) |
There was a problem hiding this comment.
Shouldn't this be called always?
There was a problem hiding this comment.
This will only be called if an error occurs outside of the callback, otherwise we deallocate the callback inside the callback when it is finished.
| const error = this.getCurrentError() | ||
| if (error.code === 0) return |
There was a problem hiding this comment.
I think if the error.code does not match errorCode, we should still throw an error (as errroCode was not 0), but make the error message unknown (could be "Unable to extract error message" or something)
| reject(new AriesAskarError(JSON.parse(this.getCurrentError()) as AriesAskarErrorObject)) | ||
| } else { | ||
| resolve() | ||
| const error = this.getCurrentError() |
There was a problem hiding this comment.
Shouldn't we also pass the errorCode here, to have the logic consistent with nodejs?
| this.code = code | ||
| this.extra = extra |
There was a problem hiding this comment.
Is extra used nowhere?
There was a problem hiding this comment.
It was not even set in the native code.
Signed-off-by: Berend Sliedrecht <[email protected]>
Signed-off-by: Berend Sliedrecht <[email protected]>
berendsliedrecht
force-pushed
the
reduce-get-error-calls
branch
from
a319774 to
531ac17
Compare
Signed-off-by: Timo Glastra <[email protected]>
get_current_error