Add functionality to get full stack trace when exceptions are thrown.

Add functionality to get full stack trace when exceptions are thrown.
This should address the issue raised in 7813.

try {
 ...
 ...
} on Object catch(e, s) {
 print(s.fullStackTrace);  // This should print the full stack trace.
}
Committed: https://code.google.com/p/dart/source/detail?r=19179

[srdjan, 2013-02-26 18:00:43]

LGTM after testing dart2js

https://chromiumcodereview.appspot.com/12316116/diff/12001/runtime/lib/error.cc
File runtime/lib/error.cc (right):

https://chromiumcodereview.appspot.com/12316116/diff/12001/runtime/lib/error....
runtime/lib/error.cc:164: // Setup a full stacktrace.
s/stacktrace/stack trace/

https://chromiumcodereview.appspot.com/12316116/diff/12001/runtime/vm/raw_obj...
File runtime/vm/raw_object.h (right):

https://chromiumcodereview.appspot.com/12316116/diff/12001/runtime/vm/raw_obj...
runtime/vm/raw_object.h:1569: return
reinterpret_cast<RawObject**>(&ptr()->pc_offset_array_);
s/pc_offset_array/catch_pc_offset_array/

[nweiz, 2013-02-26 19:17:57]

https://chromiumcodereview.appspot.com/12316116/diff/12001/sdk/lib/core/excep...
File sdk/lib/core/exceptions.dart (right):

https://chromiumcodereview.appspot.com/12316116/diff/12001/sdk/lib/core/excep...
sdk/lib/core/exceptions.dart:81: abstract class Stacktrace {
This should be called "StackTrace", and its members should be "fullStackTrace"
and "stackTrace". Currently it's inconsistent with the rest of Dart.

https://chromiumcodereview.appspot.com/12316116/diff/12001/sdk/lib/core/excep...
sdk/lib/core/exceptions.dart:84: external String get fullStacktrace;
What's the motivation for this existing separate from "stacktrace"? As I
understand the discussion in issues 4061 and 7813, the spec says that a full
stack trace is required; it doesn't mention anything about a partial stack trace
also being required, let alone being the default. I'm also not sure why a
partial stack trace would be useful.

https://chromiumcodereview.appspot.com/12316116/diff/12001/sdk/lib/core/excep...
sdk/lib/core/exceptions.dart:89: external String get stacktrace;
Is this different than toString()? If so, document how; if not, document that
fact.

[siva, 2013-02-27 02:21:59]

Per our discussion offline I have moved the stack trace interface into a
separate file called stacktrace.dart
and created stacktrace_patch.dart.

PTAL.

https://chromiumcodereview.appspot.com/12316116/diff/12001/runtime/lib/error.cc
File runtime/lib/error.cc (right):

https://chromiumcodereview.appspot.com/12316116/diff/12001/runtime/lib/error....
runtime/lib/error.cc:164: // Setup a full stacktrace.
On 2013/02/26 18:00:43, srdjan wrote:
> s/stacktrace/stack trace/

Done.

https://chromiumcodereview.appspot.com/12316116/diff/12001/runtime/vm/raw_obj...
File runtime/vm/raw_object.h (right):

https://chromiumcodereview.appspot.com/12316116/diff/12001/runtime/vm/raw_obj...
runtime/vm/raw_object.h:1569: return
reinterpret_cast<RawObject**>(&ptr()->pc_offset_array_);
Nice catch. Thank you...

On 2013/02/26 18:00:43, srdjan wrote:
> s/pc_offset_array/catch_pc_offset_array/

https://chromiumcodereview.appspot.com/12316116/diff/12001/sdk/lib/core/excep...
File sdk/lib/core/exceptions.dart (right):

https://chromiumcodereview.appspot.com/12316116/diff/12001/sdk/lib/core/excep...
sdk/lib/core/exceptions.dart:81: abstract class Stacktrace {
On 2013/02/26 19:17:58, nweiz wrote:
> This should be called "StackTrace", and its members should be "fullStackTrace"
> and "stackTrace". Currently it's inconsistent with the rest of Dart.

Done.

https://chromiumcodereview.appspot.com/12316116/diff/12001/sdk/lib/core/excep...
sdk/lib/core/exceptions.dart:84: external String get fullStacktrace;
A partial stack trace is very useful for library developers who are interested
in logging any errors encountered in their code and do not want to be bothered
by deep call chains from the callers code.
After discussion with Lars on this requirement we came up with this proposal of
providing both a full stack trace and a partial stack trace. Savvy library
developers who do not want to be bothered by deep call chains from the callers
get what they want while others get a full stack trace.

I am not sure if this needs to be specified in the Spec.

On 2013/02/26 19:17:58, nweiz wrote:
> What's the motivation for this existing separate from "stacktrace"? As I
> understand the discussion in issues 4061 and 7813, the spec says that a full
> stack trace is required; it doesn't mention anything about a partial stack
trace
> also being required, let alone being the default. I'm also not sure why a
> partial stack trace would be useful.

https://chromiumcodereview.appspot.com/12316116/diff/12001/sdk/lib/core/excep...
sdk/lib/core/exceptions.dart:89: external String get stacktrace;
Yes this different from a toString method as the toString method will get you a
full stack trace.

On 2013/02/26 19:17:58, nweiz wrote:
> Is this different than toString()? If so, document how; if not, document that
> fact.

[srdjan, 2013-02-27 18:31:56]

lgtm

[nweiz, 2013-02-27 19:07:49]

https://chromiumcodereview.appspot.com/12316116/diff/12001/sdk/lib/core/excep...
File sdk/lib/core/exceptions.dart (right):

https://chromiumcodereview.appspot.com/12316116/diff/12001/sdk/lib/core/excep...
sdk/lib/core/exceptions.dart:84: external String get fullStacktrace;
On 2013/02/27 02:21:59, siva wrote:
> A partial stack trace is very useful for library developers who are interested
> in logging any errors encountered in their code and do not want to be bothered
> by deep call chains from the callers code.
> After discussion with Lars on this requirement we came up with this proposal
of
> providing both a full stack trace and a partial stack trace. Savvy library
> developers who do not want to be bothered by deep call chains from the callers
> get what they want while others get a full stack trace.
> 
> I am not sure if this needs to be specified in the Spec.
> 
> On 2013/02/26 19:17:58, nweiz wrote:
> > What's the motivation for this existing separate from "stacktrace"? As I
> > understand the discussion in issues 4061 and 7813, the spec says that a full
> > stack trace is required; it doesn't mention anything about a partial stack
> trace
> > also being required, let alone being the default. I'm also not sure why a
> > partial stack trace would be useful.
> 

It sounds like the partial stack trace is more of a special case than the full
stack trace. What do you think about changing the field names to "stackTrace"
and "partialStackTrace" respectively?

[siva, 2013-02-27 22:43:59]

https://chromiumcodereview.appspot.com/12316116/diff/12001/sdk/lib/core/excep...
File sdk/lib/core/exceptions.dart (right):

https://chromiumcodereview.appspot.com/12316116/diff/12001/sdk/lib/core/excep...
sdk/lib/core/exceptions.dart:84: external String get fullStacktrace;
The name partialStackTrace seems to have a negative connotation, I prefer
stackTrace and fullStackTrace as it conveys the right intent.

'special case' is subjective depending on what is considered normal :-)

On 2013/02/27 19:07:49, nweiz wrote:
> On 2013/02/27 02:21:59, siva wrote:
> > A partial stack trace is very useful for library developers who are
interested
> > in logging any errors encountered in their code and do not want to be
bothered
> > by deep call chains from the callers code.
> > After discussion with Lars on this requirement we came up with this proposal
> of
> > providing both a full stack trace and a partial stack trace. Savvy library
> > developers who do not want to be bothered by deep call chains from the
callers
> > get what they want while others get a full stack trace.
> > 
> > I am not sure if this needs to be specified in the Spec.
> > 
> > On 2013/02/26 19:17:58, nweiz wrote:
> > > What's the motivation for this existing separate from "stacktrace"? As I
> > > understand the discussion in issues 4061 and 7813, the spec says that a
full
> > > stack trace is required; it doesn't mention anything about a partial stack
> > trace
> > > also being required, let alone being the default. I'm also not sure why a
> > > partial stack trace would be useful.
> > 
> 
> It sounds like the partial stack trace is more of a special case than the full
> stack trace. What do you think about changing the field names to "stackTrace"
> and "partialStackTrace" respectively?

[siva, 2013-02-27 22:48:47]

Committed patchset #4 manually as r19179 (presubmit successful).

[kasperl, 2013-04-08 14:07:52]

Surprised to see this in repository. I don't remember the API discussions at
all.

https://codereview.chromium.org/12316116/diff/5004/tests/language/full_stackt...
File tests/language/full_stacktrace1_test.dart (right):

https://codereview.chromium.org/12316116/diff/5004/tests/language/full_stackt...
tests/language/full_stacktrace1_test.dart:1: void func1() {
Copyright notice.

https://codereview.chromium.org/12316116/diff/5004/tests/language/full_stackt...
tests/language/full_stacktrace1_test.dart:13: var full_trace = s.fullStackTrace;
fullTrace

https://codereview.chromium.org/12316116/diff/5004/tests/language/full_stackt...
File tests/language/full_stacktrace2_test.dart (right):

https://codereview.chromium.org/12316116/diff/5004/tests/language/full_stackt...
tests/language/full_stacktrace2_test.dart:1: void func1() {
Copyright notice.

https://codereview.chromium.org/12316116/diff/5004/tests/language/full_stackt...
tests/language/full_stacktrace2_test.dart:13: var full_trace = s.fullStackTrace;
fullTrace

https://codereview.chromium.org/12316116/diff/5004/tests/language/full_stackt...
tests/language/full_stacktrace2_test.dart:55: print(full_trace);
fullTrace

https://codereview.chromium.org/12316116/diff/5004/tests/language/full_stackt...
File tests/language/full_stacktrace3_test.dart (right):

https://codereview.chromium.org/12316116/diff/5004/tests/language/full_stackt...
tests/language/full_stacktrace3_test.dart:1: void func1() {
Missing copyright notice?

https://codereview.chromium.org/12316116/diff/5004/tests/language/full_stackt...
tests/language/full_stacktrace3_test.dart:12: var full_trace = s.fullStackTrace;
fullTrace

https://codereview.chromium.org/12316116/diff/5004/tests/language/full_stackt...
tests/language/full_stacktrace3_test.dart:51: var full_trace = s.fullStackTrace;
fullTrace

https://codereview.chromium.org/12316116/diff/5004/tests/language/language_da...
File tests/language/language_dart2js.status (right):

https://codereview.chromium.org/12316116/diff/5004/tests/language/language_da...
tests/language/language_dart2js.status:29: full_stacktrace1_test: Skip # Stack
trace interface is not implemented yet.
It would be nice to be cc'ed -- or even get to review -- changes that have an
impact on dart2js. Who decided on the StackTrace API?

Also these should have been marked Fail not Skip, right?