search

mortenbra / alexandria-plsql-utils

Oracle PL/SQL Utility Library
786 stars 317 forks source link

flex_ws_api.make_request should re-raise an exception #28

Closed eaolson closed 6 years ago

eaolson commented 8 years ago

If flex_ws_api.make_request experiences an exception, the exception block returns null if it's because the response could not be converted to XML (I'm not sure this is a good idea) and returns without a value for other exceptions. This causes "ORA-06503: PL/SQL: Function returned without value" to be raised.

It seems to me this block should re-raise the exception for other errors.

Is returning null on a "cannot convert to XML" error the best idea? There's no longer any way to know what the server response actually was, then.

mortenbra commented 8 years ago

Hi, I agree with the issues you raise.

As noted in the package specification (https://github.com/mortenbra/alexandria-plsql-utils/blob/master/ora/flex_ws_api.pks#L8), the flex_ws_api package was originally written by Jason Straub of Oracle (http://jastraub.blogspot.com/2008/06/flexible-web-service-api.html). Jason's package eventually became the apex_web_service package that is bundled with Apex from version 4 onwards. So the (much more widely used) apex_web_service package probably suffers from the same problems as those you bring up here (might be worth testing out, though).

Be that as it may, there's no reason why we can't improve on the package as it ships with the Alexandria library.

As for the best solution, the make_request function is supposed to return an xmltype. If the server response can't be converted into valid xml, what do we return? Some kind of xml wrapper around the actual response data encoded as "cdata" (https://en.wikipedia.org/wiki/CDATA)? Or just keep on returning null for invalid xml? Or raise an exception regardless of the type of error? Suggestions are welcome.

eaolson commented 8 years ago

Huh, I did not realize this package became apex_web_service. Based on some very quick testing, I think that raises an exception in cases like this.

My vote is to raise an exception in all cases, possibly with, say, the first 1000 bytes of the response. I think the problem I was having was I was getting back "401 Unauthorized", but that was being hidden from me. I wouldn't return wrapped XML because the user may be expecting something that conforms to a particular schema.

mortenbra commented 8 years ago

Raising an exception and including parts of the response in the error text seems like a good solution to me.