From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <yew-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id D90BD1FF15E
for <inbox@lore.proxmox.com>; Tue, 6 May 2025 11:09:48 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id A29165335;
Tue, 6 May 2025 11:10:05 +0200 (CEST)
Mime-Version: 1.0
Date: Tue, 06 May 2025 11:09:31 +0200
Message-Id: <D9OYKX3EMLB3.3G43QMVLNRY3N@proxmox.com>
To: "Yew framework devel list at Proxmox" <yew-devel@lists.proxmox.com>,
"Dominik Csapak" <d.csapak@proxmox.com>
X-Mailer: aerc 0.20.1-0-g2ecb8770224a-dirty
References: <20250505132823.237431-1-s.sterz@proxmox.com>
<18b8753b-51a8-4417-9059-d291d3ec4075@proxmox.com>
In-Reply-To: <18b8753b-51a8-4417-9059-d291d3ec4075@proxmox.com>
From: "Shannon Sterz" <s.sterz@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.383 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_ASCII_DIVIDERS 0.8 Email that uses ascii formatting dividers and possible
spam tricks
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [proxmox.com, store.rs]
Subject: Re: [yew-devel] [RFC PATCH] store: use try_borrow and expect_throw
for better error messages
X-BeenThere: yew-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Yew framework devel list at Proxmox <yew-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/yew-devel>,
<mailto:yew-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/yew-devel/>
List-Post: <mailto:yew-devel@lists.proxmox.com>
List-Help: <mailto:yew-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/yew-devel>,
<mailto:yew-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Yew framework devel list at Proxmox <yew-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: yew-devel-bounces@lists.proxmox.com
Sender: "yew-devel" <yew-devel-bounces@lists.proxmox.com>
On Tue May 6, 2025 at 10:49 AM CEST, Dominik Csapak wrote:
> high level question/comment:
>
> AFAIU you want to add proper error messages to the borrows here,
> so wouldn't it be enough to use 'try_borrow().expect("some error")' ?
>
> or does the UnrwapThrowExt trait something additional that we want/need which
> I'm overlooking?
The `UnrwapThrowExt` is generally preferable for WASM usage according to
the documentation:
> These methods should have a smaller code size footprint than the
> normal `Option::unwrap` and `Option::expect` methods, but they are
> specific to working with Wasm and JS.
Hence, why I use them here instead of the ordinary `unwrap` and `expect`
functions.
>
> E.g. in PDM we have a global panic hook that displays the panic (and i think stack trace)
>
> and the default panic hook logs it into the console
> (which also happens for uncaught exceptions)
Yes and that is good practice, but adding these here instead of just
panicking without any information is probably still nicer in other usage
scenarios. So I don't really see why we shouldn't do this also.
IMO, ideally we could give components a way to try acquiring the locks
and let them handle the failure in some way. Panicking in WASM leads to
the entire GUI freezing until it is realoaded, there may be scenarios
where components can recover from not being able to write or read a
store without doing so.
One scenario I can imagine is a component that uses the information in
the Store for front-end validation. If it can't acquire a read lock on
the store, it could fail silently and rely on the backend taking care of
the validation. While this is somewhat inconvenient for users, it's
probably preferable over having to reload everything (and potentially
losing multiple inputs).
If we don't want to break the public API here, I'd suggest adding
something like `try_read()` and `try_write()`.
> On 5/5/25 15:28, Shannon Sterz wrote:
>> this makes use of the `UnwrapThrowExt` trait to improve error messages
>> when acquring store locks. taking such locks in async functions can
>> sometimes lead to bugs that can be unreliably reproduced, this should
>> provide more information about the source of the error compared to
>> just panicking.
>>
>> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
>> ---
>>
>> alternatively we could return the error here directly (or wrap it) and
>> let the call site handle such errors. however, that would require
>> breaking this public api, so i decided against this for now.
>>
>> src/state/store.rs | 13 +++++++++++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/state/store.rs b/src/state/store.rs
>> index bde7d9da..b071182c 100644
>> --- a/src/state/store.rs
>> +++ b/src/state/store.rs
>> @@ -10,6 +10,8 @@ use yew::html::IntoEventCallback;
>> use yew::prelude::*;
>> use yew::virtual_dom::Key;
>>
>> +use wasm_bindgen::UnwrapThrowExt;
>> +
>> use crate::props::{
>> ExtractKeyFn, ExtractPrimaryKey, FilterFn, IntoFilterFn, IntoSorterFn, SorterFn,
>> };
>> @@ -130,7 +132,10 @@ impl<T: 'static> Store<T> {
>> /// Panics if the value is currently mutably locked.
>> pub fn read(&self) -> StoreReadGuard<T> {
>> StoreReadGuard {
>> - state: self.inner.borrow(),
>> + state: self
>> + .inner
>> + .try_borrow()
>> + .expect_throw("Could not acquire read lock on store!"),
>> }
>> }
>>
>> @@ -142,7 +147,11 @@ impl<T: 'static> Store<T> {
>> /// When the returned [StoreWriteGuard] is dropped, the store listeners
>> /// are notified. To prevent that use [StoreWriteGuard::skip_update]
>> pub fn write(&self) -> StoreWriteGuard<T> {
>> - let state = self.inner.borrow_mut();
>> + let state = self
>> + .inner
>> + .try_borrow_mut()
>> + .expect_throw("Could not acquire write lock on store!");
>> +
>> StoreWriteGuard {
>> state,
>> update: true,
>> --
>> 2.39.5
>>
>>
>>
>> _______________________________________________
>> yew-devel mailing list
>> yew-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/yew-devel
>>
>>
>
>
>
> _______________________________________________
> yew-devel mailing list
> yew-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/yew-devel
_______________________________________________
yew-devel mailing list
yew-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/yew-devel