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

Provide more detailed on-chain error messages for failed API calls #1509

Merged
merged 4 commits into from
Oct 27, 2022

Conversation

dcroote
Copy link
Contributor

@dcroote dcroote commented Oct 26, 2022

Closes #1502.

Adds 3 possible error messages depending on the AxiosError (note the API error message is not being relayed to avoid publishing sensitive information as explained in this comment):

  • "with status code X"
  • "with no response"
  • "in building the request"

Tested with the airnode-client-dev image on Goerli with the same setup as described in #1502 and the error message now includes the 404 status code for the price request of a nonexistent coin:

API call failed with status code 404

@dcroote dcroote requested a review from a team October 26, 2022 04:12
packages/airnode-node/src/api/index.ts Outdated Show resolved Hide resolved
packages/airnode-node/src/api/index.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@amarthadan amarthadan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm leaving the wording of the error message up to you. Other than that LGTM.

@dcroote dcroote merged commit 5ad00a9 into master Oct 27, 2022
@dcroote dcroote deleted the dcroote/issue1502 branch October 27, 2022 06:34
@martinkolenic
Copy link
Contributor

Hi @dcroote, I am currently testing the issue and wanted to ask about when to expect the error in building the request and how that can be repreoduced. I'm using Beeceptor to mock responses and this works with returning a status code. Could you please advise?

I'm also thinking it could be practical to return all or at least several more error status codes (say, there's the openweather example and when you have a bad api key, it sends back an error status code so knowing that could be quite informative for the user).

@dcroote
Copy link
Contributor Author

dcroote commented Nov 2, 2022

Hi @martinkolenic - of the three error messages added ("with status code X", "with no response", and "in building the request"), the first two are the most relevant to test, with the third considered an unlikely fallback. I haven't used Beeceptor, but if you could return different status codes from the API, those should appear in the error message written on chain, like for this Goerli transaction. For the second error message, I imagine that could be tested if Beeceptor has a way to have a long delay / timeout in responding to a request.

@martinkolenic
Copy link
Contributor

Hi Derek, thanks for the information. Beeceptor does allow delayed responses and setting it to about 600 actually did the trick. Here's a formal test report:

Test 1: Check for status code messages
Environment: local eth node, local Airnode, beeceptor API response mocking

Steps: Modify the coingecko example to send requests to a mock Beeceptor endpoint, set up the endpoint to return the 404 code for any request; make request with the local Airnode; change to another code and repeat the request

Expected result: Receive more informative error messages for status codes 404, 301, 403 and 409
Actual result: Matches expected results

Test 2: Check for timeout message presence
Environment: local eth node, local Airnode, beeceptor API response mocking

Steps: Modify the coingecko example to send requests to a mock Beeceptor endpoint, set up the endpoint to return a 200 code with a 600 second response; make request with the local Airnode; wait if the request times out

Expected result: Receive a timeout message
Actual result: Matches expected results

@dcroote
Copy link
Contributor Author

dcroote commented Nov 2, 2022

@martinkolenic this is great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve on-chain error message for failed API calls
3 participants