-
Notifications
You must be signed in to change notification settings - Fork 84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add inner exception in ConvertExceptionToRuntimeException #338
Comments
This has been considered in the past and wasn't implemented as it is too
vulnerable.
I am open to ideas how to implement this in a way that doesn't put the
library users in risk.
Elad
…On Fri, Sep 3, 2021, 08:41 Balázs Somos ***@***.***> wrote:
It would be important for us not to lose the exception information as it
travels from our backend to our frontend. After analyzing the code our
suggestion would be to adjust the ConvertExceptionToRuntimeException
function to add the original exception as an inner exception to
WampRpcCanceledException and to WampRpcRuntimeException.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#338>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIS75ROLF23X3TZR5S622DUAC7A5ANCNFSM5DLVSDSA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
I think this would remain backwards compatible:
|
The issue is not backward compatibility, but having this behavior enabled
by default. This might involuntary expose internal information of the code
to an attacker, which might use it to attack the user's application.
I am open to ideas how to have this feature but also not have it enabled by
default.
Elad
…On Fri, Sep 3, 2021, 08:50 Balázs Somos ***@***.***> wrote:
I's say this would remain backwards compatible:
protected static WampException ConvertExceptionToRuntimeException(Exception exception)
{
if (exception is OperationCanceledException)
{
return new WampRpcCanceledException(null, exception.Message, null, exception.Message, exception);
}
return new WampRpcRuntimeException(null, exception.Message, null, exception.Message, exception);
}
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#338 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIS75UJEH2SBY2GP7IXV6DUADABTANCNFSM5DLVSDSA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
Hmm, good point, I see... |
Not sure what your use case is, but wouldn't it be enough to just have the exception logged on the Callee? (WampSharp supports that) |
We wanted to have the server layer between our frontend and backend the thinnest possible in our application and avoid implementing a function for every callee with an exception handling. Ideally, our backend functions would solely be called through attributed interfaces. Example:
|
It would be important for us not to lose the exception information as it travels from our backend to our frontend. After analyzing the code our suggestion would be to adjust the ConvertExceptionToRuntimeException function to add the original exception as an inner exception to WampRpcCanceledException and to WampRpcRuntimeException.
The text was updated successfully, but these errors were encountered: