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

Add redef'able var for callback on exception #3

Closed
hugoduncan opened this issue Sep 7, 2011 · 15 comments
Closed

Add redef'able var for callback on exception #3

hugoduncan opened this issue Sep 7, 2011 · 15 comments

Comments

@hugoduncan
Copy link
Contributor

I would like to make it possible to implement a restart system on top of slingshot, and to enable tool support for examining exceptions.

I think all that is needed is a to define a var containing a function which, in throw+, is passed the exception object, and possibly the &env, and can return a map containing a key to control whether the exception is thrown (and a key with a possibly modified exception object) or a value is to be returned. By default the var would be bound to a function that returned the unmodified exception to be thrown.

For example, swank could then rebdef this var if defined, to allow stack trace and locals inspection on (slingshot) exceptions without the need for JPDA support (similar in functionality to swank.core/break). Might be a nice incentive to switch to slingshot...

Thoughts?

@scgilardi
Copy link
Owner

very cool. I'll be happy to whip up an implementation based on your
description. If you have something already, please let me know.

--Steve

On Tue, Sep 6, 2011 at 10:09 PM, Hugo Duncan
[email protected]
wrote:

I would like to make it possible to implement a restart system on top of slingshot, and to enable tool support for examining exceptions.

I think all that is needed is a to define a var containing a function which, in throw+, is passed the exception object, and possibly the &env, and can return a map containing a key to control whether the exception is thrown (and a key with a possibly modified exception object) or a value is to be returned. By default the var would be bound to a function that returned the unmodified exception to be thrown.

For example, swank could then rebdef this var if defined, to allow stack trace and locals inspection on (slingshot) exceptions without the need for JPDA support (similar in functionality to swank.core/break).  Might be a nice incentive to switch to slingshot...

Thoughts?

Reply to this email directly or view it on GitHub:
#3

@scgilardi
Copy link
Owner

Please see the issue3 branch:

https://github.com/scgilardi/slingshot/commits/issue-3-implement-throw-hook

I think this is a nice layering. It's not quite what you suggested,
but I think it enables the same things. Please let me know what you
think.

Thanks!

--Steve

On Tue, Sep 6, 2011 at 10:09 PM, Hugo Duncan
[email protected]
wrote:

I would like to make it possible to implement a restart system on top of slingshot, and to enable tool support for examining exceptions.

I think all that is needed is a to define a var containing a function which, in throw+, is passed the exception object, and possibly the &env, and can return a map containing a key to control whether the exception is thrown (and a key with a possibly modified exception object) or a value is to be returned. By default the var would be bound to a function that returned the unmodified exception to be thrown.

For example, swank could then rebdef this var if defined, to allow stack trace and locals inspection on (slingshot) exceptions without the need for JPDA support (similar in functionality to swank.core/break).  Might be a nice incentive to switch to slingshot...

Thoughts?

Reply to this email directly or view it on GitHub:
#3

@hugoduncan
Copy link
Contributor Author

Looks good - putting the whole throw into the hook makes sense and seems simpler.

I would just suggest making the default creation of the the object to be thrown it's own function, so it may be reused by tools that want to display the exception object before it is thrown, but do not want to change any behaviour.

@scgilardi
Copy link
Owner

Good idea. added make-thrown and changed to capture the stack trace in the throw+ context. it ends up ignored in favor of the encapsulated stack trace for Throwables, but it's used when throwing other objects and it's now available to tools or extensions that override the default handling.

@scgilardi
Copy link
Owner

make-thrown -> make-throwable (better name)

@scgilardi
Copy link
Owner

added *catch-hook* as well. it seems to me that registering and de-registering try blocks dynamically would help a lot with a restart mechanism. thoughts?

@scgilardi
Copy link
Owner

maybe that's just another hook:

  (and *try-hook* (*try-hook* :enter (make-stack-trace))
...
  (finally
    (and *try-hook* (*try-hook* :exit (make-stack-trace)))))

@hugoduncan
Copy link
Contributor Author

The catch-hook sounds very useful too, thinking about restart bindings a
little more (it's been a while since I last used them…)

Not sure what you mean by dynamically registering try blocks…

Perhaps it is worth sketching out an implementation of restarts, perhaps
based on error-kit, so we can be confident of having a suitable interface
for the extension hooks in slingshot. I probably will not get to this for
a couple of weeks, so feel free, should you have time and inclination.

On Wed, 07 Sep 2011 12:33:58 -0400, Stephen C. Gilardi
[email protected]
wrote:

added catch-hook as well. it seems to me that registering and
de-registering try blocks dynamically would help a lot with a restart
mechanism. thoughts?

Hugo Duncan

@hugoduncan
Copy link
Contributor Author

Finally got round to looking at this. catch-hook is proving a little problematic, as it does not provide a path for the hook to return a value, which is a reasonable thing for a restart to do. Would changing the catch-hook return value to a vector containing context-map and return value be a way forward?

@hugoduncan
Copy link
Contributor Author

I pushed my initial work on restarts over slingshot to https://github.com/hugoduncan/swell

It requires the issue-3-implement-throw-hook branch of my slingshot fork, which has a commit changing the catch-hook return value to be a vector.

@scgilardi
Copy link
Owner

re: catch-hook returning a vector, I've been trying hard to keep it down to one symbol introduced into the &env seen by throws within catches. I think it would be cleaner for catch_hook to be able to modify the metadata of throw-context to request special handling. If all we need is to be able to specify a return value instead of processing the catch clauses, we could indicate that by the presence of (and value of) a :return-value key in the throw-context's metadata.

if we need more flexibility than that, we can come up with another way to encode it in the throw-context metadata.

@hugoduncan
Copy link
Contributor Author

Adding :return-value to the metadata sounds a reasonable solution to me. It might be worth adding a comment in the source about the concern re-adding symbols to &env, as it wasn't apparent, to me at least, why metadata was being used.

The doc string for catch-hook might also point out that metadata on the context map should be preserved, as any modified context map will loose the metadata unless the metadata is explicitly propogated.

@hugoduncan
Copy link
Contributor Author

Another mechanism to not pollute &env might be to use a var, and bind it. The only downside I see is the introduction of a try/finally block by binding.

@scgilardi
Copy link
Owner

Yes, I put in that comment in the extensible selectors branch but that hasn't been merged yet. I'd appreciate hearing any thoughts you have on the ideas in that branch as well. It's a breaking change to how non-predicate, non-classname selectors are specified. I think it's cleaner and allows for future compatible expansion.

--Steve

On Sep 28, 2011, at 6:57 AM, Hugo Duncan wrote:

Adding :return-value to the metadata sounds a reasonable solution to me. It might be worth adding a comment in the source about the concern re-adding symbols to &env, as it wasn't apparent, to me at least, why metadata was being used.

The doc string for catch-hook might also point out that metadata on the context map should be preserved, as any modified context map will loose the metadata unless the metadata is explicitly propogated.

Reply to this email directly or view it on GitHub:
#3 (comment)

@scgilardi
Copy link
Owner

Let's continue this discussion on the corresponding pull request which is now issue #10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants