net/url: (*Error).Timeout and (*Error).Temporary methods should use errors.As instead of type assertion

[jens1205]

The url.Error type uses type assertion in the Timeout() and Temporary() method to delegate the work to the wrapped error:

go/src/net/url/url.go

Lines 32 to 44 in 0cd9064

	func (e *Error) Timeout() bool {
	t, ok := e.Err.(interface {
	Timeout() bool
	})
	return ok && t.Timeout()
	}
	func (e *Error) Temporary() bool {
	t, ok := e.Err.(interface {
	Temporary() bool
	})
	return ok && t.Temporary()
	}

In todays time, we shouldn't have implemented the methods like this, but should have left the task of unwrapping the error until some error in the chain implements the wanted interface to the user of the error. But as these methods now are implemented, they should use errors.As to delegate the answer to Timeout() or Temporary() to the next error in the chain who implements this interface - and not only to the direct wrapped error as it is done today.

[mknyszek]

CC @neild

[neild]

Temporary is deprecated.

Timeout is superseded by specific errors such as os.ErrDeadlineExceeded or context.DeadlineExceeded. (Perhaps it's time to deprecate the Timeout method?)

Should we be making any further changes to the implementations of these methods at this time?

[jens1205]

Even if Temporary is deprecated and Timeout is about to be deprecated, the current implementation of these methods is wrong, as they are breaking errors.As.

In our codebase, we are deliberately making use of a Temporary method, where we ourselves decide which errors are "temporary". If there is an error being wrapped, and the wrapping error is implementing Unwrap just fine, with this implmentation of url.Error we still don't see with errors.As that somewhere down the error chain there is an error implmenting Temporary or Timeout.

The implementation of these two methods on net.Error is superfluous and incorrect and should not have been done. But as they are now there, even with the deprecation of Temporary and Timeout you still don't want to just delete them (which would be the right solution) as this would be a breaking change. But if you can't delete those methods, they are still active, and IMHO bugs found should be fixed.