tools.namespace

Don't catch all exceptions in read-file-ns-decl

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

Description

I spent a while trying to work out where a bug was coming from after upgrading tools.namespace. Eventually I tracked it down to an exception being thrown in parse/read-ns-decl when it calls reader/read. This was an error in tools.namespace code, not the parsing code. The error was an ArityException that was occurring because the wrong version of tools.reader was on the Classpath.

Is it desirable to catch all exceptions here, or would it be better to only catch exceptions thrown by the parsing (which seems to be the intent)? I'm not sure if it's possible to distinguish these though...

Thoughts?

Activity

Hide
Stuart Sierra added a comment -

I've never been entirely happy with the behavior of ignoring exceptions. But the alternative has been to break on any syntax error in any file. Since it's fairly common to have syntax errors during development, the trade-off was to allow tools.namespace to continue working.

In 0.3.0 I moved the try/catch one level higher from c.t.n.parse/read-ns-decl to c.t.n.file/read-file-ns-decl, specifically to detect this case.

With the tools.reader replacing clojure.core/read in tools.namespace 0.3.0, it may be possible to distinguish parsing errors from other kinds of errors.

Show
Stuart Sierra added a comment - I've never been entirely happy with the behavior of ignoring exceptions. But the alternative has been to break on any syntax error in any file. Since it's fairly common to have syntax errors during development, the trade-off was to allow tools.namespace to continue working. In 0.3.0 I moved the try/catch one level higher from c.t.n.parse/read-ns-decl to c.t.n.file/read-file-ns-decl, specifically to detect this case. With the tools.reader replacing clojure.core/read in tools.namespace 0.3.0, it may be possible to distinguish parsing errors from other kinds of errors.
Hide
Stuart Sierra added a comment -

It appears that tools.reader wraps all exceptions in ex-info with :type :reader-exception

Show
Stuart Sierra added a comment - It appears that tools.reader wraps all exceptions in ex-info with :type :reader-exception
Hide
Stuart Sierra added a comment -

fix included in 0.3.0-alpha3 release

Show
Stuart Sierra added a comment - fix included in 0.3.0-alpha3 release

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: