zopefoundation / zExceptions

zExceptions contains common exceptions used in Zope.
Other
0 stars 5 forks source link

bug in matching possible exceptions in convertExceptionType #8

Closed jugmac00 closed 2 years ago

jugmac00 commented 5 years ago

When trying to upgrade an exception, in following code... https://github.com/zopefoundation/zExceptions/blob/b0c3422abde21849d38c6cc3a64e0c3d34ea701e/src/zExceptions/__init__.py#L505 there is a lookup of the incoming exception in the zException name space.

There are a lot of exceptions listed, but also e.g. dunder methods and other, see pdp snippet.

There should be only a match with real exceptions.

(Pdb++) pp(dir(zExceptions))
['BadRequest',
 'ERROR_HTML',
 'ExceptionFormatter',
 'Forbidden',
 'HTTPAccepted',
 'HTTPBadGateway',
 'HTTPBadRequest',
 'HTTPClientError',
 'HTTPConflict',
 'HTTPCreated',
 'HTTPError',
 'HTTPException',
 'HTTPExpectationFailed',
 'HTTPFailedDependency',
 'HTTPForbidden',
 'HTTPFound',
 'HTTPGatewayTimeout',
 'HTTPGone',
 'HTTPIMUsed',
 'HTTPInsufficientStorage',
 'HTTPInternalServerError',
 'HTTPLengthRequired',
 'HTTPLocked',
 'HTTPMethodNotAllowed',
 'HTTPMovedPermanently',
 'HTTPMultiStatus',
 'HTTPMultipleChoices',
 'HTTPNetworkAuthenticationRequired',
 'HTTPNoContent',
 'HTTPNonAuthoritativeInformation',
 'HTTPNotAcceptable',
 'HTTPNotExtended',
 'HTTPNotFound',
 'HTTPNotImplemented',
 'HTTPNotModified',
 'HTTPOk',
 'HTTPPartialContent',
 'HTTPPaymentRequired',
 'HTTPPermanentRedirect',
 'HTTPPreconditionFailed',
 'HTTPPreconditionRequired',
 'HTTPProxyAuthenticationRequired',
 'HTTPRedirection',
 'HTTPRequestEntityTooLarge',
 'HTTPRequestHeaderFieldsTooLarge',
 'HTTPRequestRangeNotSatisfiable',
 'HTTPRequestTimeout',
 'HTTPRequestURITooLong',
 'HTTPResetContent',
 'HTTPSeeOther',
 'HTTPServerError',
 'HTTPServiceUnavailable',
 'HTTPTemporaryRedirect',
 'HTTPTooManyRequests',
 'HTTPUnauthorized',
 'HTTPUnavailableForLegalReasons',
 'HTTPUnprocessableEntity',
 'HTTPUnsupportedMediaType',
 'HTTPUpgradeRequired',
 'HTTPUseProxy',
 'HTTPVersionNotSupported',
 'IBadRequest',
 'IException',
 'IForbidden',
 'IHTTPException',
 'IMethodNotAllowed',
 'INotFound',
 'IRedirect',
 'InternalError',
 'MethodNotAllowed',
 'NotFound',
 'Redirect',
 'ResourceLockedError',
 'TracebackSupplement',
 'Unauthorized',
 '_HTTPMove',
 '__builtins__',
 '__cached__',
 '__doc__',
 '__file__',
 '__loader__',
 '__name__',
 '__package__',
 '__path__',
 '__spec__',
 '_compat',
 'builtins',
 'class_types',
 'convertExceptionType',
 'implementer',
 'status_reasons',
 'unauthorized',
 'upgradeException']

P.S.: Anybody knows a use case for this upgrading once string exceptions are no longer allowed?

icemac commented 5 years ago

Raising zope.publisher.NotFound in Zope 4 context leads to a zExceptions.NotFound exception which is known by Zope 4. Any other custom exception with the name of an Exception known by Zope is upgraded the same way.

It should be even possible to create a custom Exception in a PythonScript an get it converted to the proper exception here.

This whole behavior might be a feature. At least is the list of possible names definitely too long.

icemac commented 2 years ago

The next step right after accessing the exception name from the module is to check if it is an exception class:

https://github.com/zopefoundation/zExceptions/blob/b0c3422abde21849d38c6cc3a64e0c3d34ea701e/src/zExceptions/__init__.py#L505-L509

So I tend to close this issue as invalid.

dataflake commented 2 years ago

I agree