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 <sliedrecht@berend.io>
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 <sliedrecht@berend.io>
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.
It was not even set in the native code.
Signed-off-by: Berend Sliedrecht <sliedrecht@berend.io>
Signed-off-by: Berend Sliedrecht <sliedrecht@berend.io>
berendsliedrecht
force-pushed
the
reduce-get-error-calls
branch
from
a319774
to
531ac17
Compare
Signed-off-by: Timo Glastra <timo@animo.id>
get_current_error