Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backward compatibility issue of overload resolution ambiguity #2458

Open
heli-os opened this issue Jun 26, 2024 · 0 comments
Open

Backward compatibility issue of overload resolution ambiguity #2458

heli-os opened this issue Jun 26, 2024 · 0 comments
Labels
good first issue Issues that are good for first time contributors to tackle help wanted Issues we need help with tackling proposal Proposed Specification or API change

Comments

@heli-os
Copy link
Contributor

heli-os commented Jun 26, 2024

issue starts with #2170

Many code snippets are used this way.

RetryableException(
    response.status(),
    RATE_LIMIT_EXCEEDED,
    response.request().httpMethod(),
    null, // Date type (original)
    response.request(),
)

However, this code will not work with that change.

  1. get a compile time error
Overload resolution ambiguity: 
public constructor RetryableException(p0: Int, p1: String!, p2: Request.HttpMethod!, p3: Date!, p4: Request!) defined in feign.RetryableException
public constructor RetryableException(p0: Int, p1: String!, p2: Request.HttpMethod!, p3: Long!, p4: Request!) defined in feign.RetryableException
  1. ambiguous what to do when we don't want to use retry.
  • Just looking at the code in the IDE didn't work, so I came to repository.

Two solutions to solve this problem

it's not intuitive, and it creates more confusion.

// first solution
RetryableException(
    response.status(),
    RATE_LIMIT_EXCEEDED,
    response.request().httpMethod(),
    null as Long?,
    response.request(),
)
// second solution
val nonRetryable: Long? = null
RetryableException(
    response.status(),
    RATE_LIMIT_EXCEEDED,
    response.request().httpMethod(),
    nonRetryable,
    response.request(),
)

Suggest two things

  1. fundamentally fix the overload resolution ambiguity.
    • need to ideation about how to fix it.
  2. annotate how it should be written in the new constructor when don't want to retry.
    • What about adding it to a new constructor?
@kdavisk6 kdavisk6 added help wanted Issues we need help with tackling good first issue Issues that are good for first time contributors to tackle proposal Proposed Specification or API change labels Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issues that are good for first time contributors to tackle help wanted Issues we need help with tackling proposal Proposed Specification or API change
Projects
None yet
Development

No branches or pull requests

2 participants