Skip to content
This repository has been archived by the owner on Dec 23, 2023. It is now read-only.

Duration.create fails silently when given invalid arguments. #1179

Closed
sebright opened this issue May 3, 2018 · 0 comments
Closed

Duration.create fails silently when given invalid arguments. #1179

sebright opened this issue May 3, 2018 · 0 comments

Comments

@sebright
Copy link
Contributor

sebright commented May 3, 2018

Duration.create returns a "zero" value when the arguments are invalid:

public static Duration create(long seconds, int nanos) {
if (seconds < -MAX_SECONDS || seconds > MAX_SECONDS) {
return ZERO;
}
if (nanos < -MAX_NANOS || nanos > MAX_NANOS) {
return ZERO;
}
if ((seconds < 0 && nanos > 0) || (seconds > 0 && nanos < 0)) {
return ZERO;
}
return new AutoValue_Duration(seconds, nanos);
}

I think this API would be easier to use if it either truncated the values to give a value closer to what the user probably wanted or threw an IllegalArgumentException. Here are two possible ways to change the behavior:

  1. If it is important to avoid throwing exceptions, we could truncate the seconds and nanos to their minimum and maximum values. We could throw an IllegalArgumentException if the signs of the seconds and nanos are inconsistent, since it would be unclear what sign the user wanted.

  2. We could throw an IllegalArgumentException if the seconds or nanos are out of bounds or have inconsistent sign. This is more consistent with other parts of the API that enforce all class invariants with checkArgument(...).

sebright added a commit to sebright/opencensus-java that referenced this issue May 15, 2018
…arguments.

Fixes census-instrumentation#1179.  Throwing IllegalArgumentException for invalid arguments is more
consistent with the rest of the opencensus-java API.
sebright added a commit to sebright/opencensus-java that referenced this issue May 15, 2018
…arguments.

Fixes census-instrumentation#1179.  Throwing IllegalArgumentException for invalid arguments is more
consistent with the rest of the opencensus-java API.
sebright added a commit to sebright/opencensus-java that referenced this issue May 15, 2018
…arguments.

Fixes census-instrumentation#1179.  Throwing IllegalArgumentException for invalid arguments is more
consistent with the rest of the opencensus-java API.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant