217584 - feature request: Rhino: missing feature, line-number information in all thrown exceptions

Status: VERIFIED FIXED

(Rhino Graveyard :: Core, enhancement)

Description

[Merten Schumann]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.2.1) Gecko/20021130
Build Identifier: 

Hello,

is it possible to implement methods like ECMAError::getLineNumber() in
other exceptions thrown by Rhino (EvaluatorException, JavaScriptException)
too? That would be very helpful in debugging or in general when exceptions
appear. I saw EvaluatorException's message to include
   "<foo>, line 14"
which means, the line-number information was known at time of throwing the
exception. A <ex>.getLineNumber() would allow me to implement/document
a well-defined way for a user how to find out where his script crashed ...

   ~Merten

Reproducible: Always

Steps to Reproduce:





I would like to help implementing this feature by myself.
If this is possible, feel free to give me information about how to join the project.

This is just EvaluatorException.java according to ECMAException.java ...

/* -*- Mode: java; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
 *
 * The contents of this file are subject to the Netscape Public
 * License Version 1.1 (the "License"); you may not use this file
 * except in compliance with the License. You may obtain a copy of
 * the License at http://www.mozilla.org/NPL/
 *
 * Software distributed under the License is distributed on an "AS
 * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
 * implied. See the License for the specific language governing
 * rights and limitations under the License.
 *
 * The Original Code is Rhino code, released
 * May 6, 1999.
 *
 * The Initial Developer of the Original Code is Netscape
 * Communications Corporation.  Portions created by Netscape are
 * Copyright (C) 1997-1999 Netscape Communications Corporation. All
 * Rights Reserved.
 *
 * Contributor(s):
 * Norris Boyd
 *
 * Alternatively, the contents of this file may be used under the
 * terms of the GNU Public License (the "GPL"), in which case the
 * provisions of the GPL are applicable instead of those above.
 * If you wish to allow use of your version of this file only
 * under the terms of the GPL and not to allow others to use your
 * version of this file under the NPL, indicate your decision by
 * deleting the provisions above and replace them with the notice
 * and other provisions required by the GPL.  If you do not delete
 * the provisions above, a recipient may use your version of this
 * file under either the NPL or the GPL.
 */
package org.mozilla.javascript;


/**
 * The class of exceptions thrown by the JavaScript engine.
 */
public class EvaluatorException extends RuntimeException
{
 private String sourceName;
 private int lineNumber;
 private int columnNumber;
 private String lineSource;

 /**
  * Create an exception with the specified detail message.
  *
  * Errors internal to the JavaScript engine will simply throw a
  * RuntimeException.
  *
  * @param detail a message with detail about the exception
  */
 public EvaluatorException(String detail)
 {
  super(detail);
 }

 /**
  * Create an exception with the specified detail message.
  *
  * Errors internal to the JavaScript engine will simply throw a
  * RuntimeException.
  *
  * @param nativeError the Scriptable object constructed for this error.
           Scripts will get it as an argument to catch statement.
  * @param sourceName the name of the source reponsible for the error
  * @param lineNumber the line number of the source
  * @param columnNumber the columnNumber of the source (may be zero if
  *                     unknown)
  * @param lineSource the source of the line containing the error (may be
  *                   null if unknown)
  */
 public EvaluatorException(String detail, String sourceName, int lineNumber, int
columnNumber, String lineSource)
 {
  super(detail);

  this.sourceName=sourceName;
  this.lineNumber=lineNumber;
  this.columnNumber=columnNumber;
  this.lineSource=lineSource;
 }

 /**
  * Get the name of the source containing the error, or null
  * if that information is not available.
  */
 public String getSourceName()
 {
  return sourceName;
 }

 /**
  * Returns the line number of the statement causing the error,
  * or zero if not available.
  */
 public int getLineNumber()
 {
  return lineNumber;
 }

 /**
  * The column number of the location of the error, or zero if unknown.
  */
 public int getColumnNumber()
 {
  return columnNumber;
 }

 /**
  * The source of the line causing the error, or zero if unknown.
  */
 public String getLineSource()
 {
  return lineSource;
 }
}

One usage in DefaultErrorReporter.java would just be

public EvaluatorException runtimeError(String message, String sourceName,
 int line, String lineSource, int lineOffset)
{
 return new EvaluatorException(generateErrorMessage(message, sourceName, line),
sourceName, line, lineOffset, lineSource);
}

Comment 1

[Phil Schwartau]

cc'ing Igor -

Comment 2

[Igor Bukanov]

Attachment: Extending JavaScriprException and EvaluatorException to include sourcefile/line information.

The patch extends Merten Schumann suggestion to cover EvaluatorException and
JavaScriptException cases.

Now all JavaScriptException are constructed with proper source file and line
information and the only place where EvaluatorException is constructed without
it is ScriptRuntime.throwAsUncheckedException where WrappedException (a
subclass of EvaluatorException) is created. But that is fine as the method is
called to wrap reflection exceptions mostly and they are comming not from Rhino
but user-suplied Java functions.

Comment 3

[Igor Bukanov]

Merten Schumann wrote:

> I would like to help implementing this feature by myself.
> If this is possible, feel free to give me information about how to join the 
> project.

It is easy: just start sending patches (unified format is preferable) to Norris
Boyd or preferably put them into bugzilla.

Comment 4

[Igor Bukanov]

I committed the fix. If the fix does not work, the bug can be reopened.

Comment 5

[Merten Schumann]

I will get the latest drop next week and tryout the new exception features.

I've got actually an JavaScriptException which wraps an Error thrown by
junit.framework.Assert.assertEquals(), this will be a good try if there's now a
chance to get the line information.

Thanx for now!

Comment 6

[Igor Bukanov]

Attachment: Additional fix: use Context.getSourcePositionFromStack() to get source information in EvaluatorException(String detail) constructor

The initial patch would not allow to get script line that triggered
junit.framework.Assert.assertEquals() since in Rhino CVS tip
JavaScriptException is used to represent ONLY exceptions thrown by scripts via
JS throw. The errors from user-supplied Java code called via reflection would
be wrapped into WrappedException which extends EvaluatorException. 

The additional fix covers this case and would initialize source name and line
number of the exception object to the script line that called Java code.

Comment 7

[Igor Bukanov]

I committed the additional fix.

Comment 8

[Phil Schwartau]

Merten: when you try the latest source, if everything is OK,
please mark this bug "Verified". If not, you can reopen it.

Thanks!

Comment 9

[Merten Schumann]

grmpf, I think I have to wait for the latest sources to appear on the ftp server

I cannot use cvs here at my place, the ugly firewall beast prevents it
I don't have a cvs client which supports HTTP proxy or so.

Comment 10

[Merten Schumann]

YEAH, IT WORKS! :-) (finally I got CVS running through the firewall)

There's now in almost every situations at least the line number information in
JavaScriptException and WrappedException, that's very very useful.

Igor, many thanx for adding this feature!

   Merten

Comment 11

[Merten Schumann]

Just a small correction is needed in JavaScriptException IMHO

The constructor should look like

    public JavaScriptException(Object value, String sourceName, int lineNumber)
    {
        super(ScriptRuntime.toString(value));
        this.value = value;
        this.lineNumber = lineNumber;
        this.sourceName = sourceName;
    }

The lineNumber/sourceName assignments were missing ...

   ~Merten

Comment 12

[Igor Bukanov]

The correction from Merten Schumann is committed, thanks!

Comment 13

[Igor Bukanov]

Trageting as resolved against 1.5R5

Comment 14

[Merten Schumann]

Looks like there will be Rhino 1.5R5 available soon, that would be nice. :)

Thanx again for implementing this feature!