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

Source exchangerates API: clearer error message when a currency isn't supported #3677

Closed
sherifnada opened this issue May 27, 2021 · 4 comments · Fixed by #3766
Closed

Source exchangerates API: clearer error message when a currency isn't supported #3677

sherifnada opened this issue May 27, 2021 · 4 comments · Fixed by #3766
Assignees
Labels

Comments

@sherifnada
Copy link
Contributor

sherifnada commented May 27, 2021

Tell us about the problem you're trying to solve

A user reported getting the following issue when running the exchangerates API:

{"error": {"code": "base_currency_access_restricted",
"message": "An unexpected error ocurred. [Technical Support: [email protected]]"
}
}

After some investigation this worked when they removed the base currency parameter.

I suspect this happened because the free tier of exchangerates doesn’t support specifying a currency (i think they give you euro or USD data by default). The cosmetic issue is the connector does not surface a good error message.

Describe the solution you’d like

I would like the connector to surface an easily understandable message explaining that the problem is that specifying base is not supported on the free tier. This will be tricky because we need to differentiate that error from a different error like the API token being wrong. Maybe we should use HTTP request codes e.g: 403 is invalid token, other ones mean a bad base. I'm not sure exactly what the right way to identify the difference is.

Potentially one way to verify this is to make two requests in check: one without a base. If that succeeds API token is good. Then make another request with the base. If that succeeds then the issue is with the base.

This should all be documented in the check command's method because this context is not obvious.

┆Issue is synchronized with this Asana task by Unito

@sherifnada sherifnada added type/enhancement New feature or request area/connectors Connector related issues lang/python labels May 27, 2021
@sherifnada sherifnada added this to the Connectors May 28th, 2021 milestone May 27, 2021
@marcosmarxm
Copy link
Member

marcosmarxm commented May 27, 2021

From Airbyte documentation

The exchange rates integration is a toy integration to demonstrate how Airbyte works with a very simple source.

Why do not remove the base parameter?

@jrhizor
Copy link
Contributor

jrhizor commented May 27, 2021

We should hardcode the currencies supported as a regex since they're relatively static and then this problem can be avoided at input.

@cgardens
Copy link
Contributor

Could it be an enum instead of a regex?

@vitaliizazmic
Copy link
Contributor

@sherifnada you are right, Source Currency Switching is supported only in paid plans. The default base currency is EUR

vitaliizazmic added a commit that referenced this issue May 31, 2021
…de base_currency_access_restricted occurs
vitaliizazmic added a commit that referenced this issue Jun 4, 2021
…y isn't supported. Marking access_key field in spec as sensitive.

* Source Exchangerates #3677 - clearer error message when error with code base_currency_access_restricted occurs

* Source Exchangerates #3677 - improving spec

* Source Exchangerates #3675 - marked access_key as sensitive in spec

* Source Exchangerates API #3677 - improving according to PR comments

* Source Exchangerates API #3677 - change handling error info field

* Source Exchangerates API #3677 - bump version

* Source Exchangerates API #3677 - bump version (fixing merge conflicts)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment