1 2 3 Previous Next 42 Replies Latest reply on Jul 20, 2018 1:24 PM by jaramill Go to original post
      • 15. Re: Commit data only if all procedures are successful
        Cookiemonster76

        First obvious thing - never, ever write an exception handler in the procedure that logs exceptions - if that fails the only thing to do is let the whole process crash.

        For us the client logs the errors, it also logs other stuff it does, and if the client can't log then it just flat out dies, in which case you find out about (and hopefully fix) the problem very quickly. We also have monitoring on our production systems that checks disk space along with a lot else.

         

        This is of course very much in line with my "just let it die" approach.

         

        I have a vague memory (it was years ago, so don't ask for details) of some code that did a when others then log (no re-raise) and no-one was actively monitoring the log table. What happened was a particular error was slowly corrupting the data. No-one noticed for weeks because the users weren't getting an obvious error.

        By the time it was noticed the data was such a mess that fixing it was a major headache (not sure it was fixable).

         

        The downside of letting users see the error (ie. they complain about it) is far, far less than the downside of trying to deal with the mess after an actual bug has been allowed to go unchecked for an extended period of time.

         

        I could probably live with Lisandro's approach, but our clients are far more interested in expanded functionality than nixing the odd oracle error that actually appears.

        • 16. Re: Commit data only if all procedures are successful
          L. Fernigrini

          We had good DBAs , so we never ended in in out of space situation, but that needs to be taken into account.

           

          The error log procedure did not have error handling, so if it failed (never happened on the 2 years the system was in use) the error would be raised by Oracle and users (actually, the app layer) would see the full message and probably get scared.

          • 17. Re: Commit data only if all procedures are successful
            L. Fernigrini

            The problem we faced when giving the error details to the en-users ranged from

            • not understanding the language (Oracle exception messages were in English and we had users speaking Spanish and Portuguese)
            • When calling help desk, they provided part of the message, or did not know which one of the error shown was the important one.

             

            So we decided to show an error window with just a unique log #, and then that's the only thing that they need to tell the help desk. The help desk can see all the actual information logged for the error, that included the full stack and in many cases dumps from session specific values so it helped identify/reproduce the issue.

             

            And as you mentioned, the error logging mechanism did not include any error handling, if something unexpected failed there then it should be raised directly, we never had that scenario but you never know!

             

            Besides the fact that most users actually called the help desk when they received and error, there were a couple of help desk users that did some monitoring on the log in order to check that all problems were investigated.Errors logged and not reported to help desk by users were also reviewed and users sipping many errors were emailed with a message urging them to call help desk when they faced errors.

            • 18. Re: Commit data only if all procedures are successful
              Cookiemonster76

              Lisandro Fernigrini wrote:

               

              The problem we faced when giving the error details to the en-users ranged from

              • not understanding the language (Oracle exception messages were in English and we had users speaking Spanish and Portuguese)
              • When calling help desk, they provided part of the message, or did not know which one of the error shown was the important one.

              Been there, done that, bought the t-shirt

              As mentioned above errors should always be logged (either in the DB or in a client log file). If you don't it's just begging for issues like you describe.

              Only difference between your approach and mine is that you replace the error with a simple id for the user to give to support and I don't.

              But unless errors are happening a lot (and it's a lot of different errors) you don't really need an id - you can see from the log file what's going on.

              • 19. Re: Commit data only if all procedures are successful
                jaramill

                This is an interesting topic in that my latest client, the project I'm working on, the manager specifically wants the packaged procedure to fail IF certain parameters are NOT passed in or, the data values are incorrect (i.e. from date > to date).  So I created local exception variables then use DBMS_OUTPUT to display a "business error" message, then re-raise them up the food chain til the outer block then final raise has the format_error_backtrace output.  Included is the dreaded OTHERS exception to catch all in the output to the screen (which is controlled by an automated job control system).  So reading that Wiki link on ORAFAQ about WHEN OTHERS told me that i'm satisfying one of the three conditions of when (no pun intended) to use WHEN OTHERS.

                 

                Ex:

                 

                   -- These user-defined and OTHER Oracles exceptions are re-raised to make the
                   -- package procedure fail outside of Oracle (i.e. shell script)
                
                
                   EXCEPTION
                   
                      WHEN e_no_dt_time_passed THEN
                
                         DBMS_OUTPUT.PUT_LINE(DBMS_UTILITY.FORMAT_ERROR_BACKTRACE());
                         RAISE;
                
                      WHEN e_dt_time_greater_than_utc THEN
                
                         DBMS_OUTPUT.PUT_LINE(DBMS_UTILITY.FORMAT_ERROR_BACKTRACE());
                         RAISE;
                
                      WHEN e_no_from_or_to_dt_time_passed THEN
                
                         DBMS_OUTPUT.PUT_LINE(DBMS_UTILITY.FORMAT_ERROR_BACKTRACE());
                         RAISE;
                
                      WHEN e_no_region_app_found THEN
                
                         DBMS_OUTPUT.PUT_LINE(DBMS_UTILITY.FORMAT_ERROR_BACKTRACE());
                         RAISE;
                
                      WHEN e_no_region_found THEN
                
                         DBMS_OUTPUT.PUT_LINE(DBMS_UTILITY.FORMAT_ERROR_BACKTRACE());
                         RAISE;
                
                      WHEN e_no_app_found THEN
                
                         DBMS_OUTPUT.PUT_LINE(DBMS_UTILITY.FORMAT_ERROR_BACKTRACE());
                         RAISE;
                
                      WHEN OTHERS THEN
                      
                         IF(cur_1%ISOPEN) THEN
                            CLOSE cur_1;
                         END IF;
                
                
                         IF(cur_2%ISOPEN) THEN
                            CLOSE cur_2;
                         END IF;
                
                         DBMS_OUTPUT.PUT_LINE('OTHER Exception raised.' || CHR(10) || CHR(10) ||
                                              DBMS_UTILITY.FORMAT_ERROR_BACKTRACE());
                         RAISE;
                
                   END prc_main;
                
                • 20. Re: Commit data only if all procedures are successful
                  Cookiemonster76

                  Why don't you just use raise_application_error?

                  • 21. Re: Commit data only if all procedures are successful
                    Cookiemonster76

                    And really what are you trying to accomplish with those handlers?

                    You're logging to dbms_output, which means some client process has to go and retrieve that from the buffer, so why not just let the error propagate? The client will get the error stack anyway

                    • 22. Re: Commit data only if all procedures are successful
                      jaramill

                      Cookiemonster76 wrote:

                       

                      Why don't you just use raise_application_error?

                      Ahh....I forgot about that one.....https://docs.oracle.com/database/121/LNPLS/errors.htm#LNPLS99960

                      Let me look into that.

                      • 23. Re: Commit data only if all procedures are successful
                        jaramill

                        Cookiemonster76 wrote:

                         

                        And really what are you trying to accomplish with those handlers?

                        You're logging to dbms_output, which means some client process has to go and retrieve that from the buffer, so why not just let the error propagate? The client will get the error stack anyway

                        The oracle package will be called from a Unix script that is scheduled, so if any of those exceptions are raised, I want the output to be captured in a log (which I haven't seen the script for that as I'm not writing that part).

                        So are you suggesting to remove the DBMS_OUTPUT lines?

                        • 24. Re: Commit data only if all procedures are successful
                          Cookiemonster76

                          I'd suggest scrapping the exception handler entirely unless you really need to close those cursors - and if you're calling this from a unix script you probably don't.

                          If the script calls sqlplus to call stuff then any open cursors will close as soon as sqlplus exits back to the script.

                           

                          Also - if you're calling this from a scheduled script why are you checking parameters are being supplied? Aren't you in a position to know that?

                          • 25. Re: Commit data only if all procedures are successful
                            BluShadow

                            If you wrote code in our organisation that explicitly opened cursors and didn't explicitly close them, you'd get a slap.  Shoddy design and relying on implicit functions is not good.

                            • 26. Re: Commit data only if all procedures are successful
                              jaramill

                              Cookiemonster76 wrote:

                               

                              I'd suggest scrapping the exception handler entirely unless you really need to close those cursors - and if you're calling this from a unix script you probably don't.

                              If the script calls sqlplus to call stuff then any open cursors will close as soon as sqlplus exits back to the script.

                              Well I am explicitly closing the cursors that I explicitly opened but JUST in case an exception is raised, I check to see that IF they are still open then close them.  And this is common practice I've seen in past projects so I understood it and follow it.

                              Most of the times I use CURSOR-FOR LOOPS which in that case I don't check if it's open to then close in the execution or exception sections.

                               

                              Cookiemonster76 wrote:

                               

                              Also - if you're calling this from a scheduled script why are you checking parameters are being supplied? Aren't you in a position to know that?

                              In my last project, we (the developers) also created the job in UC4 (aka Automic) so yes I would be in a position to know that.  Right now I don't know (but will ask my manager).  But there are times when someone will manually run this from the command line via SQL* Plus in whatever O/S (Windows, Unix) so I need to have some business error message displayed.  I did though look at RAE (Raise App Error) as you suggested and using that as it's intended for user-defined exceptions.

                              • 27. Re: Commit data only if all procedures are successful
                                Cookiemonster76

                                I might be being a bit unconcerned since the only explicit cursors I ever use are ref cursors and if there's an error processing those then it's the consumer (client) that needs to close them. My ref cursor procedures are invariably:

                                BEGIN

                                OPEN ref FOR ....

                                END;

                                 

                                Should I worry about the open erroring in a way that leaves the cursor open? Seems to be taking defensive coding to the extreme to me.

                                As does worrying about common garden variety explicit cursors - they are scoped to the procedure/block they're declared in. If oracle doesn't clean them up when the procedure errors out that would be an oracle bug.

                                Now if you're talking about files (utl_file) then I would absolutely make sure they are closed on error.

                                And if I was writing code as the consumer of a ref cursor I would make sure to close it on error. (though if I'm writing DB code that consumes a ref cursor my first question is why?)

                                 

                                To me it depends on what those cursors in Jaramills code actually are.

                                • 28. Re: Commit data only if all procedures are successful
                                  Cookiemonster76

                                  If people can run this procedure manually from sqlplus then you absolutely don't want to use dbms_output since you can't be sure they've run set serveroutput on.

                                  Use raise_application_error.

                                  • 29. Re: Commit data only if all procedures are successful
                                    jaramill

                                    Cookiemonster76 wrote:

                                     

                                     

                                    To me it depends on what those cursors in Jaramills code actually are.

                                    Here's an example of one of the cursors that I explicitly open (slightly altered to not put in my actual code)

                                     

                                    CURSOR cur_1(param IN table.column%TYPE) IS
                                       SELECT tca.region
                                         FROM dev_applications tca
                                        WHERE 1 = 1
                                          AND tca.app = cur_1.param;