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

Add error catch to results of backend response #12

Merged
merged 1 commit into from
Mar 10, 2023
Merged

Conversation

fridgepoet
Copy link
Member

@fridgepoet fridgepoet commented Mar 6, 2023

Why do we need this feature?
In the context of the reported issue, with redshiftAsyncQueryDataSupport = false, the error runs through: https://github.com/grafana/grafana/blob/9e1ea8c990d4d52d3db518ec6e3e75422730be3a/packages/grafana-runtime/src/utils/DataSourceWithBackend.ts#L231

This PR provides the same error handling in order to trigger the same error display.

Which issue(s) does this PR fix?:
Contributes to grafana/redshift-datasource#209
Need to release this in a bumped version, bump the async-query-data versions in Redshift (and Athena) and release Redshift (and Athena) to fix the reported issue.

Special notes for your reviewer:
Local testing:

  1. In OSS Grafana repo, reproduce the original issue with the following in custom.ini
[feature_toggles]
redshiftAsyncQueryDataSupport = true
  1. In Redshift repo's package.json, bump the version of the "@grafana/async-query-data" in the dependencies object to "0.1.4",
    It should read:
  "dependencies": {
    "@grafana/async-query-data": "0.1.4",
    "@grafana/experimental": "^0.0.2-canary.39"
  },

(This step is not strictly required but has unified the version of async-query-data in Redshift and made the following step easier to manage. This version will be bumped again in Redshift and Athena after this PR is merged, so I don't think it's a big deal to do this locally for testing purposes...)

  1. In this repo grafana-async-query-data-js, pull these changes. yarn build
  2. In Redshift repo, delete any dist and node_modules to start from a clean state. Build the backend withmage. For the frontend, yarn install first. Copy the dist from grafana-async-query-data-js repo and replace the dist in the node_modules in Redshift repo.

Screenshot 2023-03-07 at 10 53 00

  1. In Redshift repo, complete the frontend build with yarn dev or yarn build.
  2. In OSS Grafana, build the frontend (yarn install, yarn start) and start backend server (make run)
  3. In your browser, reproduce the original issue. This is what it should look like with the changes from this PR:
    Screenshot 2023-03-07 at 10 56 37

Huge thanks to @kevinwcyu for all the help in finding this one

@fridgepoet fridgepoet requested a review from a team as a code owner March 6, 2023 17:23
@fridgepoet fridgepoet requested review from iwysiu and idastambuk and removed request for a team March 6, 2023 17:23
@fridgepoet fridgepoet marked this pull request as draft March 6, 2023 17:42
@fridgepoet fridgepoet marked this pull request as ready for review March 7, 2023 09:57
@fridgepoet fridgepoet merged commit 51e3085 into main Mar 10, 2023
@fridgepoet fridgepoet deleted the catchError branch March 10, 2023 09:08
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.

None yet

2 participants