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

Calling __refresh while refreshing #20576

Closed
jupl opened this issue Jan 13, 2020 · 12 comments
Closed

Calling __refresh while refreshing #20576

jupl opened this issue Jan 13, 2020 · 12 comments
Labels
help wanted Issue with a clear description that the community can help with. status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. type: bug An issue or pull request relating to a bug in Gatsby

Comments

@jupl
Copy link

jupl commented Jan 13, 2020

Description

Our team is leveraging the Gatsby refresh endpoint (ENABLE_GATSBY_REFRESH_ENDPOINT) for development purposes. The amount of time to build/refresh has increased over time. We are wanting to use a service that will make a POST call to __refresh on demand. The problem that we are encountering is that if you make the POST call in the middle of a refresh then unexpected behavior starts to occur and eventually the dev server crashes. It would be nice to have some way to either offer some kind of throttling or at least have a way to know if we are in the middle of a refresh. (with the latter at least a custom endpoint can be created)

Our understanding was that it did not behave like this before (2.16.5) and if this were to occur Gatsby would wait until the existing refresh would finish.

Steps to reproduce

Make a POST call to __refresh while it's refreshing.

Expected result

Expected behavior.

Actual result

Query failures, crashes, etc.

Environment

(This also occurs in Windows)

EDIT: Updated gatsby to 2.18.21 and it is still an issue.

  System:
    OS: macOS Mojave 10.14.6
    CPU: (8) x64 Intel(R) Core(TM) i7-3615QM CPU @ 2.30GHz
    Shell: 5.3 - /bin/zsh
  Binaries:
    Node: 12.12.0 - /usr/local/bin/node
    Yarn: 1.19.1 - /usr/local/bin/yarn
    npm: 6.11.3 - /usr/local/bin/npm
  Languages:
    Python: 2.7.16 - /usr/local/bin/python
  Browsers:
    Chrome: 79.0.3945.117
    Firefox: 72.0
    Safari: 13.0.4
  npmPackages:
    gatsby: ^2.18.15 => 2.18.15
    gatsby-background-image: ^0.9.11 => 0.9.11
    gatsby-image: ^2.2.4 => 2.2.36
    gatsby-plugin-lodash: ^3.1.2 => 3.1.18
    gatsby-plugin-manifest: ^2.2.1 => 2.2.33
    gatsby-plugin-polyfill-io: ^1.1.0 => 1.1.0
    gatsby-plugin-prefetch-google-fonts: ^1.4.3 => 1.4.3
    gatsby-plugin-react-helmet: ^3.0.12 => 3.1.18
    gatsby-plugin-remove-serviceworker: ^1.0.0 => 1.0.0
    gatsby-plugin-robots-txt: ^1.5.0 => 1.5.0
    gatsby-plugin-sass: ^2.1.12 => 2.1.26
    gatsby-plugin-sharp: ^2.2.3 => 2.3.7
    gatsby-plugin-sitemap: ^2.2.9 => 2.2.24
    gatsby-plugin-styled-components: ^3.0.7 => 3.1.16
    gatsby-plugin-typescript: ^2.0.15 => 2.1.22
    gatsby-plugin-webpack-bundle-analyser-v2: ^1.1.5 => 1.1.8
    gatsby-source-filesystem: ^2.1.2 => 2.1.42
    gatsby-transformer-sharp: ^2.2.1 => 2.3.9
  npmGlobalPackages:
    gatsby-cli: 2.6.7
@sidharthachatterjee
Copy link
Contributor

This is interesting. I suppose the correct fix here would be to queue refresh calls and only run the next one once the queue is empty?

@pieh Thoughts on this?

@sidharthachatterjee sidharthachatterjee added the type: bug An issue or pull request relating to a bug in Gatsby label Jan 16, 2020
@pieh
Copy link
Contributor

pieh commented Jan 16, 2020

Yes! Let's implement queue for that! There also other places I wanted simple queue like that (particularly our redux state persistence which is now just debounced)

@github-actions
Copy link

github-actions bot commented Feb 6, 2020

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here.
If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

@github-actions github-actions bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Feb 6, 2020
@github-actions
Copy link

Hey again!

It’s been 30 days since anything happened on this issue, so our friendly neighborhood robot (that’s me!) is going to close it.
Please keep in mind that I’m only a robot, so if I’ve closed this issue in error, I’m HUMAN_EMOTION_SORRY. Please feel free to reopen this issue or create a new one if you need anything else.
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks again for being part of the Gatsby community! 💪💜

@pieh pieh added not stale and removed stale? Issue that may be closed soon due to the original author not responding any more. labels Feb 16, 2020
@pieh pieh reopened this Feb 16, 2020
@elweab
Copy link

elweab commented Mar 17, 2020

@pieh Hey I'm wondering if there has been any progress on this issue?

@pieh pieh added the help wanted Issue with a clear description that the community can help with. label Mar 17, 2020
@pieh
Copy link
Contributor

pieh commented Mar 17, 2020

There wasn't any progress yet on this, but it's up for the taking.

The code for refresh endpoint is in

/**
* Refresh external data sources.
* This behavior is disabled by default, but the ENABLE_REFRESH_ENDPOINT env var enables it
* If no GATSBY_REFRESH_TOKEN env var is available, then no Authorization header is required
**/
const REFRESH_ENDPOINT = `/__refresh`
const refresh = async (req: express.Request): Promise<void> => {
let activity = report.activityTimer(`createSchemaCustomization`, {})
activity.start()
await createSchemaCustomization({
refresh: true,
})
activity.end()
activity = report.activityTimer(`Refreshing source data`, {})
activity.start()
await sourceNodes({
webhookBody: req.body,
})
activity.end()
}
app.use(REFRESH_ENDPOINT, express.json())
app.post(REFRESH_ENDPOINT, (req, res) => {
const enableRefresh = process.env.ENABLE_GATSBY_REFRESH_ENDPOINT
const refreshToken = process.env.GATSBY_REFRESH_TOKEN
const authorizedRefresh =
!refreshToken || req.headers.authorization === refreshToken
if (enableRefresh && authorizedRefresh) {
refresh(req)
}
res.end()
})

What we need is to make sure that calling refresh is delayed if there is already one in progress. There might be some trickiness with passing webhookBody - if there are 2 refresh requests with webhookBody we would need to make sure we actually call sourceNodes with both bodies serially

@xavivars
Copy link
Contributor

Even a simple "reject" would be better that the current behavior...

@apaniel
Copy link

apaniel commented Jun 16, 2020

@pieh hope #24887 fits what you had in mind for this fix

@LekoArts LekoArts added status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. and removed not stale labels Jan 10, 2022
@sensedrive
Copy link

Still a problem ... and it keeps falling on our team's feet.

I'm honestly a bit disappointed why there is no solution to this problem after two years. Also no response to a two year old merge request.

I would really like to know why a bug of a core feature of Gatsby is treated this way.

@Rbcoder1
Copy link

I am interested to fix this ?

@Rbcoder1
Copy link

what is the current state of this issue is their any change or not.
So , I fork this !

@LekoArts
Copy link
Contributor

Hi!

I'm closing this as a stale issue as in the meantime Gatsby 5 and related packages were released. You can check our Framework Version Support Page to see which versions currently receive active support. If this is a feature request, please create a discussion as we moved feature requests from issues to GitHub Discussions.

Please try the mentioned issue on the latest version (using the next tag) and if you still see this problem, open a new bug report. It must include a minimal reproduction.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issue with a clear description that the community can help with. status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants