racket / drracket

DrRacket, IDE for Racket
http://www.racket-lang.org/
Other
445 stars 93 forks source link

#lang errortrace doesn't work well in DrRacket #564

Closed sorawee closed 1 year ago

sorawee commented 2 years ago

With "debugging" being on, running

#lang errortrace racket

(error "a")

results in:

exception raised by error display handler: car: contract violation
  expected: pair?
  given: '#(3-unsaved-editor 3 0 26 11); original exception raised: a
  context...:
   .../private/map.rkt:40:19: loop
   /Users/soraweep/projects/racket/extra-pkgs/errortrace/errortrace-lib/errortrace/errortrace-lib.rkt:294:0: print-error-trace
   /Users/soraweep/projects/racket/extra-pkgs/errortrace/errortrace-lib/errortrace/errortrace-lib.rkt:394:4: errortrace-error-display-handler
   body of 'anonymous-module
   /Users/soraweep/projects/racket/extra-pkgs/drracket/drracket/drracket/private/stack-checkpoint.rkt:114:30
   /Users/soraweep/projects/racket/extra-pkgs/drracket/drracket/drracket/private/stack-checkpoint.rkt:83:0: call-with-stack-checkpoint
...

With "debugging" being off, running

#lang errortrace racket

(error "a")

results in:

exception raised by error display handler: vector-ref: contract violation
  expected: vector?
  given: '((error "a") 3-unsaved-editor 3 0 26 11); original exception raised: a
  context...:
   /Users/soraweep/projects/racket/extra-pkgs/drracket/drracket/drracket/private/stack-checkpoint.rkt:178:0: errortrace-stack-item->srcloc
   /Users/soraweep/projects/racket/extra-pkgs/drracket/drracket/drracket/private/stack-checkpoint.rkt:341:0: pick-first-defs
   /Users/soraweep/projects/racket/extra-pkgs/drracket/drracket/drracket/private/stack-checkpoint.rkt:598:0: get-exn-source-locs
   /Users/soraweep/projects/racket/racket/collects/racket/contract/private/arrow-val-first.rkt:486:18
   /Users/soraweep/projects/racket/extra-pkgs/drracket/drracket/drracket/private/debug.rkt:344:2: error-display-handler/stacktrace
   body of 'anonymous-module
...
sorawee commented 2 years ago

More info: it looks like DrRacket's stack-checkpoint expects cms to contain vectors, and when "debugging" is on, something would make cms contain vectors (I don't know what "something" is).

But normally, cms would contain lists, and everywhere else also expects a list.


Consider:

#lang errortrace racket

(require errortrace/errortrace-key)

(continuation-mark-set->list
 (exn-continuation-marks
  (with-handlers ([exn:fail? values])
    (error "a")))
 errortrace-key)

In DrRacket, with debugging being on, it results in:

'(#(3-unsaved-editor 8 4 159 11)
  #(3-unsaved-editor 7 2 119 52)
  #(3-unsaved-editor 6 1 93 79)
  #(3-unsaved-editor 5 0 63 126))

However, in (1) DrRacket with debugging being off and (2) plain Racket, it results in:

'(((error "a") 3-unsaved-editor 8 4 159 11)
  ((with-handlers ((exn:fail? values)) (error "a")) 3-unsaved-editor 7 2 119 52)
  ((exn-continuation-marks (with-handlers ((exn:fail? values)) (error "a"))) 3-unsaved-editor 6 1 93 79)
  ((continuation-mark-set->list
    (exn-continuation-marks (with-handlers ((exn:fail? values)) (error "a")))
    errortrace-key)
   3-unsaved-editor
   5
   0
   63
   126))
sorawee commented 1 year ago

@rfindler can I submit a PR to revert https://github.com/racket/drracket/commit/f4c495b27e5db5e9b5ef40c878514f3eb21ecbc2? It causes this bug. Do you recall why you made the change?

rfindler commented 1 year ago

I've pushed some stuff here and here. Still need to do more testing and also while this program:

#lang errortrace racket/base
(+ 1
   (+ 2
      (+ 3
         #f)))

doesn't signal an errors, it also doesn't look like the right error message yet (when run in DrRacket, I mean).

sorawee commented 1 year ago

What is the error message that you got? It looks right to me, but perhaps I missed something.

Screen Shot 2022-07-10 at 9 46 57 AM
rfindler commented 1 year ago

Oh, that's true! I do see that. Just the text in the REPL however, isn't great:

Welcome to DrRacket, version 8.6.0.1--2022-07-08(346a6c4e/d) [cs].
Language: errortrace racket/base, with debugging; memory limit: 128 MB.
. . +: contract violation
  expected: number?
  given: #f
  errortrace...:

The stuff after the line "errortrace" is gone.

sorawee commented 1 year ago

My understanding is that it's a heuristic by DrRacket to suppress the textual stacktrace (and instead show them in the stacktrace dialog). You would know it better than I do :-). In any case, my thought is that improving that could be separate from this bug fix.

rfindler commented 1 year ago

I guess I just need to track down what's going on as I just found it plain confusing!

sorawee commented 1 year ago

IIRC, it's specifically exn:fail that has this trimming. exn would not be trimmed. From a quick grep it's probably https://github.com/racket/drracket/blob/master/drracket/drracket/private/debug.rkt#L421

rfindler commented 1 year ago

Oh, I think that's something else. DrRacket makes the ellipsis-based lines clickable, hiding their content until clicked. But there is no actual content in this one, as far as I can tell.

sorawee commented 1 year ago

I'm pretty sure the ... button would have worked fine if there's no

Interactions disabled: errortrace/lang/body does not support a REPL (no #%top-interaction)

I tried:

#lang racket

(raise-argument-error 'a "a" 0 2 3)

This generates:

a: contract violation
  expected: a
  given: 2
  argument position: 1st
  other arguments...:

and the ... button works as expected.

Now, create file.rkt with:

#lang racket

(provide (except-out (all-from-out racket)
                     #%top-interaction))

and change the first program to:

#lang s-exp "file.rkt"

(raise-argument-error 'a "a" 0 2 3)

Suddenly, the ... button no longer works!

We should definitely make ... work when

Interactions disabled: errortrace/lang/body does not support a REPL (no #%top-interaction)

appears, but #lang errortrace should also support the REPL. This is done in https://github.com/racket/errortrace/pull/34

sorawee commented 1 year ago

@rfindler I think you didn't mean to push https://github.com/racket/drracket/commit/15a3c0610e49404d5b24b5993be90a7ed22b890e to the master branch. That commit depends on the errortrace change, which is still not pushed yet.

rfindler commented 1 year ago

Oh, sorry! That was a mistake. I've reverted it for now. I want to run all the tests (and do the docs on the errortrace side) first.