In general in exception handling is basic rule, handle only exception which you know and know how to avoid or inform user better then general exception handler know it. So calling function which might raise exception is common thing and it is handled on place where you have enough information about effect of exceptions so instead of immediate catch of IO.open exception you can catch in e.g. function which call load_configuration. Then you know that you don't have configuration due to some well known problems (so never catch all exception unless you must unrelevant problems e.g. in code). So you catch IO exception when you call load configuration handle it (use default or print to users) and you still can log information about exception. So to simplify it to simple rules: - always catch only limited exception, generic one is caught by global exception catcher. - catch when you know effects of problem so you can qualify handle it (like load default etc.) - never catch exception only to log problem ( sometime I see it in webyast, it is horrible as it just duplicates code and generic handler log it), if you do not want to ignore it. - always log exception if you do not rethrow it So for Klaus examples: 1) it is all functions as each function raise exception, so only if you know exception and can handle in expected way 2) Good one if you in catch handle it 3) it is not DRY. It duplicates function of global exception catch point and also in some case it is error as it can hide problems and program can start behave strange without any visible reason. (like current System, which just write system cannot boot without anything. It just confuse and hides potential problem.) Josef Martin Vidner write:

On Thu, Apr 22, 2010 at 11:36:58AM +0200, Klaus Kaempf wrote:

...

when calling functions known to raise exceptions, its always good practice to enclose calls with begin ... rescue ... end and catch exceptions locally.

Good that you bring it up because I think that you got it absolutely wrong.

Unless you give more specific examples, this only creates more problems by bypassing the generic, well-thought out system-wide recue from.

Your code will hide error details, or cause unnecessary code duplication.

...

Catching exceptions should cover known problems (i.e. missing permissions) as well as generic problems.

Example:

WRONG:

call_function_which_might_raise

CORRECT, unknown exceptions are handled by exception_trap in http://gitorious.org/opensuse/yast-web-client/blobs/master/webclient/app/con...

...

NOT ENOUGH:

begin call_function_which_might_raise rescue KnownException => e handle_known_exception e end

debatable

...

GOOD:

begin call_function_which_might_raise rescue KnownException => e handle_known_exception e rescue Exception => e handle_generic_exception e end

WRONG, as I said first

...

The 'wrong' and 'not enough' cases validate bug reports.

I don't understand what you mean by this. Bug reports are validated by the developer reproducing them.

-- Josef Reidinger YaST team maintainer of perl-Bootloader, YaST2-Repair, parts of webyast -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org For additional commands, e-mail: yast-devel+help@opensuse.org

Klaus Kaempf write:

* Martin Vidner [Apr 22. 2010 12:05]:

...

On Thu, Apr 22, 2010 at 11:36:58AM +0200, Klaus Kaempf wrote:

...

when calling functions known to raise exceptions, its always good practice to enclose calls with begin ... rescue ... end and catch exceptions locally.

Good that you bring it up because I think that you got it absolutely wrong.

Unless you give more specific examples, this only creates more problems by bypassing the generic, well-thought out system-wide recue from.

Yes, you're right. I stand corrected.

...

CORRECT, unknown exceptions are handled by exception_trap in http://gitorious.org/opensuse/yast-web-client/blobs/master/webclient/app/con...

My problem is rest-service, not web-client.

Apparently the exception trap in rest-service is not sufficient.

See bnc#598794 where the rest-service errors out (presumably) because of a permission error but this isn't shown in the rest-service logs.

This should be already fixed, what version of rest-service do you use? ( I add logging of all exceptions)

And the web-client interpretes this as 503 Service Unavailable without any hints about the real cause.

I think that problem is that miso catch too generic exception in users, because out global catch handler if he gets 503 and in response is reported insufficient permissions then it redirect to control panel and correctly print message. Also clientException class has ability to print real cause in his message method, because it understand out backend exceptions. You can try it with e.g. tux user in appliance on almost all modules. Reason why we use 503 and not 4** is that we want explicitelly know which error code is thrown by lighttpd and which one is from our ruby code. Our ruby code should always throw 503 and serialize information about problem in xml, where is more informations. So problem is not uncaught exception, but maybe somewhere other. How can I reproduce this behavior? Josef

Klaus --- SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)

-- Josef Reidinger YaST team maintainer of perl-Bootloader, YaST2-Repair, parts of webyast -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org For additional commands, e-mail: yast-devel+help@opensuse.org

Klaus Kaempf write:

* Josef Reidinger [Apr 22. 2010 14:33]:

...

...

See bnc#598794 where the rest-service errors out (presumably) because of a permission error but this isn't shown in the rest-service logs.

This should be already fixed, what version of rest-service do you use? ( I add logging of all exceptions)

origin/master from around noon.

...

So problem is not uncaught exception, but maybe somewhere other. How can I reproduce this behavior?

I guess its related to a missing org.opensuse.yast.modules.yapi.groups permission. Without details (and missing details are exactly my current problem) I cannot say more :-(

I try to simulate it now and it works as expected. Show red flash with report of insufficient permissions. I check rest-service code of groups and I find different code when DBus::Error occure so I report it. But I cannot find why your problem is not at least logged.

Klaus --- SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)

-- Josef Reidinger YaST team maintainer of perl-Bootloader, YaST2-Repair, parts of webyast -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org For additional commands, e-mail: yast-devel+help@opensuse.org

Klaus Kaempf write:

* Josef Reidinger [Apr 22. 2010 15:25]:

...

I try to simulate it now and it works as expected. Show red flash with report of insufficient permissions.

Right, I read "Permission to allow org.opensuse.yast.modules.yapi.users.groupsget is not available for user yastws"

Funny thing is that sudo polkit-auth --user yastws --grant org.opensuse.yast.modules.yapi.users.groupsget fails with polkit-auth: AuthorizationAlreadyExists: An authorization for uid 423 for the action org.opensuse.yast.modules.yapi.users.groupsget with constraint '' already exists

So we still have a policykit problem hidden somewhere ?!

No, it is well known issue if you run webyast as other user then yastws. I hardcoded yastws user if policyKit throw missing permission, because only yastws run it in production.

...

I check rest-service code of groups and I find different code when DBus::Error occure so I report it.

I fixed GroupsController and ApplicationController meanwhile to catch and report DBus::Error once.

...

But I cannot find why your problem is not at least logged.

Yeah, thats actually the core of bnc#598794

If it him global exception catch, then it must be logged. So something between catch it and return response without logging content.

Klaus --- SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)

-- Josef Reidinger YaST team maintainer of perl-Bootloader, YaST2-Repair, parts of webyast -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org For additional commands, e-mail: yast-devel+help@opensuse.org

Hi, ähm, as I am often running into this problem which exception has to be handled or not, what is the result of this discussion ? Greetings Stefan Am 22.04.2010 16:35, schrieb Klaus Kaempf:

* Josef Reidinger [Apr 22. 2010 16:26]:

...

Klaus Kaempf write:

...

So we still have a policykit problem hidden somewhere ?!

No, it is well known issue if you run webyast as other user then yastws. I hardcoded yastws user if policyKit throw missing permission, because only yastws run it in production.

Ah, this defeats the purpose of allowing developers to run WebYaST locally ;-)

What about replacing hardcoded 'yastws' with Etc.getlogin ?

...

...

...

I check rest-service code of groups and I find different code when DBus::Error occure so I report it.

I fixed GroupsController and ApplicationController meanwhile to catch and report DBus::Error once.

...

But I cannot find why your problem is not at least logged.

Yeah, thats actually the core of bnc#598794

If it him global exception catch, then it must be logged. So something between catch it and return response without logging content.

I'm currently trying to come up with a test case.

Klaus --- SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)

-- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org For additional commands, e-mail: yast-devel+help@opensuse.org