ErrorDecoder and retry-after header timezones issue

[freddiN]

I believe there is a timezone issue in the the core/src/main/java/feign/codec/ErrorDecoder.java when parsing the retry-after header in the "public Date apply(String retryAfter)" method.

The code in the mentioned class basically does:

public static void main(String[] args) throws ParseException {
    final DateFormat RFC822_FORMAT = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss 'GMT'", Locale.US);
    Date d = RFC822_FORMAT.parse(RETRY-HEADER);
}

I'm in the timezone Europe/Berlin, here a "Mon, 14 Aug 2023 13:32:06 GMT" retry-after header becomes a "Mon Aug 14 13:32:06 CEST 2023" Date object .

Unfortunately the RetryableException that is then thrown and handled by my customized Retryer just contains the Date object and not the raw response header, so I cannot fix it by parsing the retry-after header again.

I created a local workaround with a custom errordecoder (the convert functrion is the workaround itself, going through ZonedDateTime and invoking Instant):

public class CustomErrorDecoder implements ErrorDecoder {
    @Override
    public Exception decode(String methodKey, Response response) {
        FeignException exception = feign.FeignException.errorStatus(methodKey, response);
        String retryAfter = firstOrNull(response.headers(), RETRY_AFTER);
        if (retryAfter != null) {
            return new RetryableException(
                    response.status(),
                    exception.getMessage(),
                    response.request().httpMethod(),
                    exception,
                    convert(retryAfter),
                    response.request());
        }
        return exception;
    }

    private Date convert(String retryAfter) {
        try {
            ZonedDateTime zoned = ZonedDateTime.parse(retryAfter, DateTimeFormatter.RFC_1123_DATE_TIME);
            return Date.from(zoned.toInstant());
        } catch(Exception e) {

        }

        return null;
    }

    private <T> T firstOrNull(Map<String, Collection<T>> map, String key) {
        if (map.containsKey(key) && !map.get(key).isEmpty()) {
            return map.get(key).iterator().next();
        }
        return null;
    }
}

[vitalijr2]

Could you make a PR for this issue, please?

j.u.Date doesn't contain any timezone data, it uses Calendar to resolve milliseconds to timezonned date-time string, see javadoc https://docs.oracle.com/javase/8/docs/api/java/util/Date.html

To avoid it you can set GMT timezone for your application if it's possible, e.g.
TZ=GMT java -jar app.jar....

Unfortunately toGMTString is deprecated.

[freddiN]

Hi Vitalij,

That doesnt fix the issue: The app is already fully aware of the timezone it got from the OS, but DateFormat.parse() still emits a broken Date() object in a non-GMT environment.

Unfortunately feigns RetryableException is still using a pre-java8 java.util.Date object, but with the workaround above at least the Date object is created correctly (tested in a GMT container as well as GMT+2).

Unfortunately toGMTString is deprecated.

Imho the whole class should be deprecated. The "new" java.time classes are around since java 8 (2014) and resolve a lot of issues developers have regarding timezones.

[vitalijr2]

I suppose we can make breaking change to replace date with seconds: we don't use Date itself, every time we operate getTime().