Update core 1.151.2 #4373
Update core 1.151.2 #4373
Conversation
| @@ -493,7 +493,7 @@ If you think that's a bug and you need that permission, then please open an issu | |||
| accountId, | |||
| msgId, | |||
| update, | |||
| description | |||
| '' | |||
| ) | |||
There was a problem hiding this comment.
It's a bit strange to set the parameter as deprecated, while at the same time we have to pass it to core to avoid an error there...
Should at least be an optional parameter in the type definitions in jsonrpc
There was a problem hiding this comment.
Should at least be an optional parameter in the type definitions in jsonrpc
jsonrpc type was probably forgotten to be updated?
https://github.com/deltachat/deltachat-core-rust/blob/dc5fcdf4255cadd8f44c0e723c95713ffb877986/deltachat-jsonrpc/src/api.rs#L1770
writing Option<String> there would let you pass null, though you still need to fill that parameter because of deltachat/yerpc#10 (otherwise ts will complain) -> so not I would say it isn't worth to change.
So I would have removed the property from jsonrpc, and mention it in changelog as breaking change.
typescript provides type checking so desktop would be fine, python/go bots might not be, but adb tests them every time he updates core anyway, so it would be watched before production, even if changelog is not read. But currently its important that the new release comes fast, so I understand playing it safe in such times.
There was a problem hiding this comment.
Maybe I add this Option, since I'm adapting jsonrpc right now anyway, since it still misses dc_msg_get_webxdc_href() which is needed for #4366
| 'parameter description in sendUpdate is deprecated and will be removed in the future' | ||
| ) | ||
| } | ||
| ipcRenderer.invoke('webxdc.sendUpdate', JSON.stringify(update)) |
There was a problem hiding this comment.
Don't we want to return this promise?
Hmm, but the new types state that we return void
There was a problem hiding this comment.
This was not changed in this PR
There was a problem hiding this comment.
Umm, I don't think so.
(update, description) =>
ipcRenderer.invoke(
'webxdc.sendUpdate',
JSON.stringify(update),
description
)returns a promise.
There was a problem hiding this comment.
It was changed in this pr, but the type in types definition is void, so in that sense it is correct.
You should normally expect to wait for it to be sent out, because you get you own updates in the listener.
But internally it is async and if we don't await on it, possible errors will currently always trigger "uncaught errors / unhandled promise rejection".
This might be a specification issue in error handling then: if we say that sendUpdate can fail in core and it should report errors.
No description provided.