#91 avoid except clauses without exception specification
Opened 7 years ago by pavlix. Modified 5 years ago

Each time I am debugging any issues with fedpkg, I end up not seeing tracebacks despite -d and -v options. In most cases this is due to catch-all except clauses.

$ grep -r except:
src/pyrpkg/cli.py:        except:
src/pyrpkg/__init__.py:        except:
src/pyrpkg/__init__.py:        except:
src/pyrpkg/__init__.py:            except:
src/pyrpkg/__init__.py:        except:
src/rpkg:    except:

$ grep -r 'except Exception'
src/pyrpkg/cli.py:        except Exception:
src/pyrpkg/cli.py:        except Exception:
src/pyrpkg/cli.py:        except Exception as e:
src/pyrpkg/cli.py:        except Exception as e:
src/pyrpkg/cli.py:        except Exception as e:
src/pyrpkg/__init__.py:        except Exception as e:
src/pyrpkg/__init__.py:        except Exception as e:
src/pyrpkg/__init__.py:        except Exception:
src/pyrpkg/__init__.py:        except Exception as e:
src/pyrpkg/__init__.py:        except Exception as e:
src/pyrpkg/__init__.py:            except Exception as err:
src/pyrpkg/__init__.py:            except Exception as err:
src/pyrpkg/__init__.py:        except Exception:
src/pyrpkg/__init__.py:                except Exception as error:
src/pyrpkg/__init__.py:                except Exception as error:
src/pyrpkg/lookaside.py:            except Exception as e:
src/pyrpkg/lookaside.py:            except Exception as e:
src/pyrpkg/lookaside.py:            except Exception as e:

I'm pretty sure that not all of the exceptions above are re-raised or printed for the user.


Not all exceptions are needed to be re-raised, and some of them are re-raised as an instance of rpkgError. I choose one except randomly, for example this one

src/pyrpkg/cli.py: except:

Its full context is

1579         try:
1580             client.args.path = os.getcwd()
1581         except:
1582             print('Could not get current path, have you deleted it?')
1583             sys.exit(1)

and this one

src/pyrpkg/init.py:296: except:

 289         try:
 290             if anon:
 291                 self._anon_kojisession = koji.ClientSession(defaults['server'],
 292                                                             session_opts)
 293             else:      
 294                 self._kojisession = koji.ClientSession(defaults['server'],
 295                                                        session_opts)
 296         except:
 297             raise rpkgError('Could not initiate %s session' %
 298                             os.path.basename(self.build_client))

and also this one

src/pyrpkg/init.py:2149: except Exception:

2146         try:
2147             repoid = self.anon_kojisession.getRepo(
2148                 build_target['build_tag_name'])['id']
2149         except Exception:
2150             raise rpkgError('Could not find a valid build repo')

Can you list the exceptions you think that should be re-raised but not?

It is IMO better to always catch the specific exception you need to catch. Otherwise bad things can happen like unexpected exceptions being used for control flow instead of printing a traceback that could then be used to analyze and fix a bug.

Is there any catch-all except clause that is actually needed? Or are they all just out of laziness to specify the proper exception type?

This issue has been unresolved for more than a year, and is going to be closed within a week if no further action is taken. If you feel this is in error, please contact me.
This is a cleaning process suggested by Jay Greguske. Copy of this ticket was already closed in JIRA tracker.

Login to comment on this ticket.

Metadata