Hello,
is it o.k that we get ERROR-STATUS yes and NUM-MESSAGES = 0?
And why ERROR-STATUS is no when we remove NO-ERROR?
Short example:
define variable LocRetText as character no-undo.
input from value("C:\windows\system.ini").
repeat:
import unformatted LocRetText no-error.
end.
input close.
message "done" error-status:error error-status:num-messages view-as alert-box.
Hi Stefan,
Using no-error indicates to the ABL that you're going to handle the errors yourself using error-status:error.
However if you don't not use it then the ABL will throw an error which can be handled using a CATCH block otherwise the default error handling will occur.
Not too sure why num-messages would be 0 though.
Hello,
In this short test I think that correct will be return error-status:error = No if execute in Windows machine, but for me return Yes.
Some explication for this?
If ERROR-STATUS is yes there should be a message so NUM-MESSAGES should be > 0.
If you remove the NO-ERROR, the value of ERROR-STATUS is based on the last time NO-ERROR was used, so it really could be anything.
But I understand - based on the fact that when you did use NO-ERROR, ERROR-STATUS was yes, when you remove it you would expect to see an error message displayed. And you didn't. I assume this goes along with the fact that NUM-MESSAGES was 0. Something is wrong there. Sounds like ERROR-STATUS being Yes is a bug.
If you use RETURN ERROR, then ERROR-STATUS:ERROR will be yes and ERROR-STATUS:NUM-MESSAGES 0!
If you change code to check for ERROR-STATUS just after line with NO-ERROR excample seems to work!
repeat:
import unformatted LocRetText no-error.
IF ERROR-STATUS:ERROR THEN MESSAGE "ERROR" VIEW-AS ALERT-BOX.
end.
So follow the recommendation and check for ERROR-STATUS:ERROR just after the line with NO-ERROR!
yes, It seems a bug to me, if num-messages is 0 then error-status:error should be no.
Even when i tried removing no-error and thought let ABL handle it, then ABL is not throwing any error as well.
Yes, that is always the best way to do it regardless - check immediately after the statement. Otherwise, you may miss errors that occurred during the repeat loop. You would only see be checking what happened the last time through. But since there is no RETURN statement here, it is still a mystery why NUM-MESSAGES was 0. But thanks for that reminder!
Even more funny, if real error inside repeat, it seems to have removed the num-messages after the END! :(
So seems someone is tampering with error status in the 'END.' of repeat loop!
DEFINE VARIABLE i AS INT64 NO-UNDO.
define variable LocRetText as character no-undo.
input from value("C:\windows\system.ini").
repeat:
import unformatted LocRetText no-error.
i = INT("a") NO-ERROR.
end.
input close.
message "done" ERROR-STATUS:ERROR ERROR-STATUS:NUM-MESSAGES view-as alert-box.
I believe this behavior is specific to input statements within repeat blocks. Essentially the error that raises when there is no more to input from your file is used to force the repeat to drop without specifying a leave statement.
The num-messages is zero because this error is "expected". The reason there is no message for your i = INT("a") is because the repeat drops before hitting that line on the final iteration (because the finished input forces a leave).
At least this is my understanding of it. I hope this helps.
Our real production issue was that this is the last code in a procedure and the calling code checks ERROR-STATUS and it's still true.
And I think the ERROR-STATUS may not "escape" out of an iteration (EACH) block.
Is this really expected? I can't find any doc or KB about it.
Shall I raise a support call?
A quick test seems to show that the same behavior applies to a for each block, the end of an input statement will force a leave and set ERROR-STATUS:ERROR = TRUE and NUM-MESSAGES = 0.
It may well be a bug, especially since it gives misleading information. By "expected" i don't mean to suggest it's intended, it seems everyone agrees NUM-MESSAGES should never be 0 if ERROR-STATUS true.
But seriously. We've all already agreed that you should check for an error immediately after using NO-ERROR. If you do that, it works as expected. So I would say - be happy with that! I didn't check, but what David said sounds "right" (meaning it is a valid explanation) and we will probably not change this.
When the import statement reach the end of the file, it raises the END-KEY condition, which leaves the loop. It is not an error, it is expected (and desired) behaviour that causes the loop undo and leave once all work was done on the file.
Thus I do not expect an error or a message when you remove the NO-ERROR. Further more, the ERROR-STATUS handle relates to the last statement with a NO-ERROR, so if there is no NO-ERROR, referencing it make no sense and it can in fact lead to misleading results in a large program or system.
The fact that ERROR-STATUS:ERROR is true in the example does seem to me like a bug, since we do not expect an error when the end of the file is reached. We expect the END-KEY condition to be raised, which should undo and leave the repeat loop. It is not an error, it is desired and expected and was designed to work that way.
Laura,
very seriously:
If you would try it on your own then you will notice that there isn't any error except EOF.
I created a special test file with one line with LF/CR (because win) at the end and used this code:
define variable LocRetText as character no-undo.
input from value("C:\temp\OneLine.txt").
repeat on error undo, retry:
if retry then do:
message "oops"
view-as alert-box.
leave.
end.
message "read line"
view-as alert-box.
import unformatted LocRetText no-error.
message LocRetText error-status:error error-status:num-messages
view-as alert-box.
end.
input close.
message "done" error-status:error error-status:num-messages view-as alert-box.
Result:
read Line
A line for Laura 0 no
read line
done yes 0
In summary:
import unformatted LocRetText no-error seems to set error-status where isn't any because
there is no "oops" message without no-error.
When I remove no-error, it works as expected:
read line
A line for Laura no 0
read line
done no 0
Please don't tell me that I could simple remove no-error, that isn't the point.
In general in the whole 15 years with OE 4GL I missing a "REPEAT UNTIL EOF INPUT"
This would be a smoother option instead ENDKEY emulation for EOF that seems not to work in a correct way.
A perfect workaround could be that:
file-info:file-name = "C:\temp\OneLine.txt".
define variable LocRetText as character no-undo.
input from value(file-info:file-name).
repeat on error undo, retry:
if retry then do:
message "oops"
view-as alert-box.
leave.
end.
if seek(input) = file-info:file-size then leave.
import unformatted LocRetText no-error.
end.
input close.
message "done" error-status:error error-status:num-messages view-as alert-box.
Result: done no 0
I am going to open a ticket because it's a bug and it looks like I am not alone with my opinion.
Thank you all for your remarks.
Stefan
But seriously, you are missing my point. I was already agreeing with you that the "ERROR" was coming from the EOF. In fact, I even agree that this is a bug!
But this is easily fixed, not by the work-around you showed, but by simply removing the check for ERROR-STATUS:ERROR outside of the REPEAT block. The value in the MESSAGE inside the REPEAT block always says the correct thing. The last time, when you get EOF, while it sets ERROR-STATUS:ERROR, it also kicks you out of the block. So you never get to that MESSAGE statement (in this case) that looks at it.
So to repeat what I said above, you should check for an error immediately after using NO-ERROR. If you do that, it works as expected. Immediately, means immediately - the next line. Not after the END statement following.
I have no objection to you logging a bug, but I'm trying to be honest with you. I think that it will get low priority. 1. It is a behavior change which is always problematic for us. 2. There is a simple "work-around", which isn't even a work-around, it is just coding correctly based on the way that NO-ERROR is designed to work.
Ah. I see you already have logged a bug, and marked it high priority! Please comment on my last post to see if we can agree on a priority for this.
Hi Laura,
i don't have acces to the log system, the case level is 3 (question/inconvenice)
Planned for 12.0 (currently)
I couldn't open it my self because the web ticket system had a bug too this morning :-)
I will propose my colleague to remove no-error because we don't want to know any error.
The check of error-status was added only to identify the bug (cause)why the persistent procedure with this code returned ERROR-STATUS:Error to the calling program.
Stefan
Laura,
We will definitely consider your suggestion about setting both the ReturnValue and the error message. I don't think this change can break any existing code. Do you? Then we can also have the AppError constructor with the single parameter also set both since basically everyone is confused by that! And your point about NUM-MESSAGES also makes sense. Thanks.
While we are talking about ERROR-STATUS and its behavior, I would like to request an enhancement - in an attempt to bridge the gap between the old style error handling and THROW / CATCH. Consider in this example that we have an error that was thrown by some new code in the system, but you need to intercept and react on it, but then re-throw the current error. This is not possible, because once you use NO-ERROR, the error object is lost to you. Consider the sample code, after the error was thrown, of what we would like to achieve:
RUN someInternalProc NO-ERROR.
IF ERROR-STATUS:ERROR
THEN
DO:
someSystemPersistentObject:cleanOutProperties().
IF VALID-OBJECT(ERROR-STATUS:ERROR-OBJECT)
THEN RETURN ERROR ERROR-STATUS:ERROR-OBJECT.
ELSE RETURN ERROR RETURN-VALUE.
END.
Out of past experience, I know that I should preserve the error-object / return-value as these could be influenced by code in cleanOutProperties(), but hopefully, having the error-object, I could preserve it. The reason it is important to be able to have access to the error-object is to be able to get at the :CallStack property. Even if I cannot preserve the error-object, I could get the CallStack and re-throw a new error object, where I embed the stack (or part thereof), like what was done with the Dynamics error include.
...or alternatively, add an STACK-TRACE property to ERROR-STATUS that can be populated from the CallStack property of an error-object.
This is perhaps a better approach...?
And why wouldn't you just use a catch block? Are you only interested in an error from one specific line? Even then you can use DO ON ERROR UNDO, LEAVE/CATCH around that line.
You have to turn it on using a startup parameter or a session attribute. ERROR-STACK-TRACE or something like that. Read the doc. It's in there!
We did that for performance reasons.
Hi Laura,
I agree with you that we could take the approach of wrapping everything in undo-able blocks, but I don't like the fact that we have to introduce even more blocks, which themselves introduce overhead. We have tens (if not hundreds) of thousands of lines of legacy code - and not necessarily the capacity to rework it at the moment. This was written long before structured error handling existed. Back in the day, error-handling was not nice. If you left off the NO-ERROR on a statement, OpenEdge would pop a message to a screen (GUI) or write a message to a log file, and then return to the caller that there was NO error. ERROR-STATUS:ERROR would be false and you had nothing in ERROR-STATUS:GET-MESSAGE(x) either. This could lead to disastrous results if something didn't "throw" the error back (or even knew and error occurred). You had to be manually "throw" your errors as follows (Somewhat simplified):
RUN doSomething NO-ERROR.
IF ERROR-STATUS:ERROR
THEN
DO:
/* Note that we would give preference to something in RETURN-VALUE */
IF RETURN-VALUE <> ? AND RETURN-VALUE <> "":U
THEN RETURN ERROR RETURN-VALUE.
ELSE RETURN ERROR ERROR-STATUS:GET-MESSAGE(1) + "|" + "Build up your call stack manually here".
END.
When you execute code, you can get back an application generated error (RETURN ERROR "SomeError") or an error raised by the ABL (mismatched parameteers, etc.). To have any idea of where the error occurred, you had to build in the call stack as part of your RETURN-VALUE, so that developers knew where to go and find the problem. This is demonstrated with the code above. The code in the error handling block was placed in an include to ensure consistency. So you now have something like:
RUN doSomething NO-ERROR.
{ReturnError.i}
The problem with having an error thrown back with the THROW/CATCH style error-handling is that the legacy code intercepts the error, but we have no way of getting at the original stack of where the error occurred. Therefore, we cannot patch the include file to include the correct call stack. So, as code gets modernized down the line, legacy code has an issue we can't cater for.
I hear that you mention that the startup parameter was introduced for performance reasons. I have heard this previously from someone at Progress. What is the impact? I cannot imagine that you would not want to leave it on, so where is the performance issue introduced? In every statement that gets executed or only when you encounter an error? Surely it should be the latter, because that is the only point you are interested in getting the call stack? And if so, that shouldn't really matter, because you only hit this when you run into an error - which should be far less than your actual number of lines of code executed.
Laura,
Re Henri's post:
Re the call stack and performance: If you need the call stack information just turn the feature on and don't worry about it. You are correct, that this only affects performance when an error occurs, not on every statement. And you're correct that probably you would never notice this performance hit in the schema of all the other stuff that is going on. Perhaps we were being overly cautious, but sometimes we get bitten when we aren't!
Re your original request about having the error object available on ERROR-STATUS: I think I got it now. I believe you want this so that new code that uses CATCH/THROW inter-operates well with legacy code that uses NO-ERROR. But just so I'm totally clear on your use-case, let's be explicit. Seems like there are 2 cases:
1. If someone does a THROW using new structured error handling, the legacy code that handles it with NO-ERROR can then access the call stack from the original thrown error object. However, this implies that you'd change all the legacy code to check for this. I assume you could change your ReturnError.i to do this?
2. The legacy code that uses NO-ERROR can trap a Progress error and the error object would be available off of ERROR-STATUS. So again, how would you handle this in your ReturnError.i? Would you change this into a THROW of the object? Or would you just read the call stack out of the object and add it to your RETURN ERROR string, avoiding the need for you to put the call stack together yourself?
Actually, not sure how difficult that would be for you guys to implement but adding an error-object property to the error-status system handle sounds like a great idea… then existing properties/methods (error-num, error-status, get-message) can simply use that object if valid. It will be the same as before, this is only set when no-error is used on a statement and it keeps on till the next statement with no-error. That way non-structured error handling can get access to the call-stack (as requested by Henri, I think) and we fix the AppError ;)
Hi Laura,
Thanks for your response. Because of the include file (ResetError.i), we will be pretty much able to adapt it any way you would be able to accommodate us. I think I would prefer to re-throw the error, because I have access to it if you were able to add it to ERROR-STATUS. If scoping of the object was an issue, having the CALL-STACK available would also help me achieve my goal. So, I would settle for anything you would be able to provide that would help. :) Having said that, Marian has some sound ideas around how to potentially handle the life-cycle of the object and when to clean up. And it also sounds like having the object would benefit him too.
Thank you Laura and Marian for taking out the time to discuss the issue. It is much appreciated.
By chance stumbled on QUERY-PREPARE documentation.
Here ERROR-STATUS:ERROR is FALSE but NUM-MESSAGES is 1.
So guess it is not un-common that ERROR-STATUS:ERROR is FALSE and NUM-MESSAGES > 0
This isn't really about QUERY-PREPARE per se. It is just that QUERY-PREPARE is a handle method. Like many handle methods, if there is an error, the AVM does not raise error but treats it like a warning. So you do get an error message but ERROR-STATUS:ERROR is false.
We were not happy about this behavior but couldn't just change it wholesale or we would break people's code. But we did modify it when we introduced structured error handling. If your block now uses structure error handling, this will behave differently. It WILL raise error and ERROR-STATUS:ERROR will be true if you use NO-ERROR. Or if you have a CATCH block the error will be caught.
Having structured error handling on the block means either that you have a CATCH block or that the UNDO, THROW option is in effect for the block either because it is coded on the block (ON ERROR UNDO, THROW), or you are using either ROUTINE-LEVEL ON ERROR UNDO, THROW or BLOCK-LEVEL ON ERROR UNDO, THROW and it applies to the block you are in.