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

dnsdist: allow known exception types to be converted to string #6541

Merged
merged 1 commit into from May 24, 2018

Conversation

chbruyand
Copy link
Member

Short description

This tries to improve Lua error reporting, see #6535. Exceptions thrown in a Lua subroutine can be printed:

x = newNMG()
res, err = pcall(function() x:addMask('banana') end)
if not res then
   print("An error has been thrown: ", err)
end

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled and tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

@chbruyand
Copy link
Member Author

For the record, we could also catch some specific exception and convert them to lua_error directly in the Lua wrapper : https://gist.github.com/chbruyand/586820716b128cf680d12599fd46adc7

This allows to get more details about the error

res, err = pcall(function() throwSomeException() end)
if not res then
   print (err)
end

will output

[string "chunk"]:1: Caught exception: Something bad happened

But it would also require to catch PDNSExceptionss as they diverge from std::exceptions, which would not be so clean.

@zeha
Copy link
Collaborator

zeha commented Apr 30, 2018

But it would also require to catch PDNSExceptionss as they diverge from std::exceptions, which would not be so clean.

Maybe its time to ditch PDNSException or make it a std subclass.

@ahupowerdns
Copy link
Contributor

looks useful!

@pieterlexis pieterlexis merged commit a8e6fcb into PowerDNS:master May 24, 2018
@chbruyand chbruyand deleted the lua-error-msg branch May 24, 2018 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants