Potential issue with multiple registrations of same handler in error hint

Currently the experimental api for error hint registration allows same handler to be registered multiple times. And the handler is called multiple times when triggered.

Bear with me on this not so minimal example:

using Markdown
using StyledStrings
struct MemberAccessException <: Exception
	objType::DataType
	x::Symbol
end

Base.getproperty(d::Dict, x::Symbol) = begin
	if x in propertynames(d)
		return getfield(d, x)
	else
		throw(MemberAccessException(typeof(d), x))
	end
end

function Base.showerror(io::IO, exc::MemberAccessException)
	@nospecialize
	println(io, "MemberAccessError: member access of this field on this DataType is not entertained.\n")
	Base.Experimental.show_error_hints(io, exc)
end

function memberAccessHandler(io, exc)
	@nospecialize
	x = exc.x
	objType = exc.objType
	if objType <: Dict
		println(io, 
		"""
		The field `$x` is not a member of Dict type. In case you
		are trying to access values using keys consider
		using `indexing` operation. 
		""")
		println(io, styled"{bold:  Example:}")
		println(io, styled"{bold:  ≡≡≡≡≡≡≡}")
		print(io, styled"{bold, green:  julia> }")
		println(io, "dict = Dict(:$(x)=>1)")
		print(io, styled"{bold, green:  julia> }")
		println(io, "dict[:$(x)]")
	else
		println(io, 
		"""
		This datatype does not allow member access of this field -`$(x)` for end users.
		Please refer to documentation on $(objType) on access rights and means to do it.
		"""
		)
	end
end


Base.Experimental.register_error_hint(memberAccessHandler, MemberAccessException)
Base.Experimental.register_error_hint(memberAccessHandler, MemberAccessException)
Base.Experimental.register_error_hint(memberAccessHandler, MemberAccessException)

h = Dict(:a=>5)

h.c
julia> Base.Experimental._hint_handlers
IdDict{Type, Vector{Any}} with 4 entries:
  MemberAccessException => [memberAccessHandler, memberAccessHandler, memberAccessHandler]
  MethodError           => [noncallable_number_hint_handler, nonsetable_type_hint_handler, string_concatenati…
  UndefVarError         => [UndefVarError_hint]
  FieldError            => [fielderror_hint_handler]

_hint_handlers stored multiple memberAccessHandler handlers.

From my perspective this should not allowed. I see two options:

  1. Check if this handler exists already and push! only if its not already registered.
  2. replace existing handler with new handler.

@Lilith I came across this while experimenting with error hint for Dict. Do you find this reasonable?

If you register a handler multiple times then it triggers multiple times? That sounds reasonable.

For development workflows you can register a handler and then overwrite that function’s methods at the REPL or via Revise.jl

For deployment, don’t call register_error_hint multiple times with the same arguments.

1 Like

Yes. I agree with you. When I had this concern in my head, I was thinking from package developers perspective. If we have a very common exception like FieldError, then I was worried developers would register their own version of hint handlers; and we will end up having multiple error hints. May be I am not thinking straight.

You are right that if multiple packages register error hints, then multiple handlers may get called when an error is thrown. I think that is acceptable.

For example, if Dictionaries.jl has an error hint when someone triggers a FieldError on a Dictionary, and Base has a hint for Dicts, then both handlers will be called on all field errors, which should have little performance impact and print at most one error because nothing is both a Dict and a Dictionary.

1 Like

got it.