Allow passing custom error buffer to wasm_instance_new_with_args #1526
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is helpful if the user uses different logging technique and wants to get error message back to their system
Collaborator
|
In my opinion, I don't suggest to change signature of any released public API. I always tend to create a new one. But in this case, I believe we should use |
Collaborator
|
i agree it's better to use a trap, at least for a subset of errors. |
Collaborator
Author
|
makes sense, resolving as we already have #1751 |
wenyongh
pushed a commit
that referenced
this pull request
Nov 25, 2022
Create trap for error message when wasm_instance_new fails: - Similar to [this PR](#1526), but create a wasm_trap_t to output the error msg instead of adding error_buf to the API. - Trap will need to be deleted by the caller but is not a breaking change as it is only created if trap is not NULL. - Add error messages for all failure cases here, try to make them accurate but welcome feedback for improvements. Signed-off-by: Andrew Chambers <[email protected]>
NingW101
pushed a commit
to NingW101/wasm-micro-runtime
that referenced
this pull request
Dec 1, 2022
…lliance#1751) Create trap for error message when wasm_instance_new fails: - Similar to [this PR](bytecodealliance#1526), but create a wasm_trap_t to output the error msg instead of adding error_buf to the API. - Trap will need to be deleted by the caller but is not a breaking change as it is only created if trap is not NULL. - Add error messages for all failure cases here, try to make them accurate but welcome feedback for improvements. Signed-off-by: Andrew Chambers <[email protected]>
wenyongh
pushed a commit
that referenced
this pull request
Dec 6, 2022
Create trap for error message when wasm_instance_new fails: - Similar to [this PR](#1526), but create a wasm_trap_t to output the error msg instead of adding error_buf to the API. - Trap will need to be deleted by the caller but is not a breaking change as it is only created if trap is not NULL. - Add error messages for all failure cases here, try to make them accurate but welcome feedback for improvements. Signed-off-by: Andrew Chambers <[email protected]>
wenyongh
pushed a commit
that referenced
this pull request
Dec 19, 2022
Create trap for error message when wasm_instance_new fails: - Similar to [this PR](#1526), but create a wasm_trap_t to output the error msg instead of adding error_buf to the API. - Trap will need to be deleted by the caller but is not a breaking change as it is only created if trap is not NULL. - Add error messages for all failure cases here, try to make them accurate but welcome feedback for improvements. Signed-off-by: Andrew Chambers <[email protected]>
vickiegpt
pushed a commit
to vickiegpt/wamr-aot-gc-checkpoint-restore
that referenced
this pull request
May 27, 2024
…lliance#1751) Create trap for error message when wasm_instance_new fails: - Similar to [this PR](bytecodealliance#1526), but create a wasm_trap_t to output the error msg instead of adding error_buf to the API. - Trap will need to be deleted by the caller but is not a breaking change as it is only created if trap is not NULL. - Add error messages for all failure cases here, try to make them accurate but welcome feedback for improvements. Signed-off-by: Andrew Chambers <[email protected]>
g0djan
pushed a commit
to g0djan/wasm-micro-runtime
that referenced
this pull request
Sep 30, 2024
…lliance#1751) Create trap for error message when wasm_instance_new fails: - Similar to [this PR](bytecodealliance#1526), but create a wasm_trap_t to output the error msg instead of adding error_buf to the API. - Trap will need to be deleted by the caller but is not a breaking change as it is only created if trap is not NULL. - Add error messages for all failure cases here, try to make them accurate but welcome feedback for improvements. Signed-off-by: Andrew Chambers <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is helpful if the user uses different logging technique and wants to get error message back to their system
This breaks public API and I'm not sure how do we proceed with this kind of changes (assuming you're happy to accept it). Could you please provide a recommendation or point me to a process related to releasing backward-incompatible changes? Optionally we can make the last two arguments default (e.g. set
error_buftoNULLand if that's the case, thewasm_instance_new_with_argswill use internal buffer) however I find this a bit polluting the API, so would rather avoid default arguments in that case.