Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

provide more info on script errors #358

Closed
pq opened this issue May 22, 2018 · 7 comments
Closed

provide more info on script errors #358

pq opened this issue May 22, 2018 · 7 comments

Comments

@pq
Copy link
Collaborator

@pq pq commented May 22, 2018

From @pq on May 22, 2018 22:36

currently (at least some) script errors provide very little information to the REPL (e.g., ### SCRIPT ERROR and nothing more). if we're trapping the error it'd be great to pass more back (in the repl or log).

a few observations:

  • load failures do produce output
  • "null pointers" don't

to reproduce, put this into an init() and play:

local a = nil
print(a.tostring())

result:

# script run
# script init
### SCRIPT ERROR:

Copied from original issue: ngwese/maiden#82

@pq
Copy link
Collaborator Author

@pq pq commented May 22, 2018

/cc @catfact

@pq
Copy link
Collaborator Author

@pq pq commented May 22, 2018

From @catfact on May 22, 2018 22:38

norns.scripterror() is called with no arguments here:
https://github.com/catfact/norns/blob/dev/lua/script.lua#L41

this should probably use norns.try (as far as i can tell)

this functionality seems messy and error-prone to me - interdependency on script.lua, menu.lua and norns.lua

@pq
Copy link
Collaborator Author

@pq pq commented May 22, 2018

From @ngwese on May 22, 2018 22:39

i think this would be better filed against matron in the norns repo - maiden has no control over this as the repl is just echos whatever is sent over the socket

@pq
Copy link
Collaborator Author

@pq pq commented May 22, 2018

@catfact should this get moved to the norns repo?

@pq
Copy link
Collaborator Author

@pq pq commented May 22, 2018

From @catfact on May 22, 2018 22:39

yes i agree

@pq
Copy link
Collaborator Author

@pq pq commented May 22, 2018

From @catfact on May 22, 2018 22:39

this stuff is also printed to the norns screen... sometimes? maybe?

@antonhornquist
Copy link
Collaborator

@antonhornquist antonhornquist commented May 23, 2018

agreed this is important. must have been a quite recent change causing lua errors not to be reported in detail - there used to be stack traces.

pq added a commit that referenced this issue Jun 4, 2018
fixes: #358

(there's another `pcall` in `cleanup()` that could maybe get similar treatment, but since there's a `norns.try` right after maybe that's intended?)

/cc @tehn @catfact
@pq pq closed this in #398 Jun 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants