Skip to content

Prevent double fetch. - #67

Merged
buckett merged 2 commits into
masterfrom
double-fetch-prevent
Jan 30, 2026
Merged

buckett merged 2 commits into
masterfrom
double-fetch-prevent

Conversation

@buckett

@buckett buckett commented Jan 28, 2026

Copy link
Copy Markdown
Member

When using <React.StrictMode> the useEffect fires twice which causes the token to be loaded twice. Because the server only allows the token to be loaded once we have to ensure that if we've already fetched the token we don't do it again.

When using <React.StrictMode> the useEffect fires twice which causes the token to be loaded twice. Because the server only allows the token to be loaded once we have to ensure that if we've already fetched the token we don't do it again.
Copilot AI review requested due to automatic review settings January 28, 2026 10:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR prevents double fetching of LTI tokens when using React StrictMode by implementing a ref-based guard in the useEffect hook. In development mode with StrictMode enabled, useEffect runs twice, which was causing the token fetch to fail on the second attempt since the server only allows a token to be retrieved once.

Changes:

  • Added hasFetchedRef using useRef to track whether token has been fetched
  • Added guard in useEffect to prevent multiple fetches
  • Added test case to verify behavior in StrictMode
  • Minor formatting updates (trailing commas)

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/components/tokenRetriever/LtiTokenRetriever.tsx Implements ref-based guard to prevent duplicate token fetches in StrictMode
src/components/tokenRetriever/LtiTokenRetriever.test.jsx Adds test to verify only one fetch occurs in StrictMode
Comments suppressed due to low confidence (1)

src/components/tokenRetriever/LtiTokenRetriever.tsx:95

  • The useEffect has ltiServer in its dependency array, but the hasFetchedRef.current guard prevents the effect from running more than once. This creates an inconsistency: if ltiServer changes after the initial mount, the effect will be triggered but won't actually execute the fetch due to the ref check.

Since this component is designed to fetch a one-time token only once (as mentioned in the comments), the dependency array should be empty. Change [ltiServer] to [] to accurately reflect that this effect should only run on mount, regardless of prop changes.

  useEffect(() => {
    if (hasFetchedRef.current) return;
    hasFetchedRef.current = true;
    const fetchToken = async () => {
      const token = getToken();
      const server = getServer();

      if (!token) {
        setState({ loading: false, error: "No id found to load token with" });
        return;
      }

      if (!server) {
        setState({ loading: false, error: "No server found to load from" });
        return;
      }

      try {
        const formData = new FormData();
        formData.append('key', token);

        const response = await fetch(`${server}/token`, {
          method: 'POST',
          body: formData,
        });

        if (!response.ok) {
          if (response.status === 403) {
            throw new Error("Sorry the tool is not currently available to you.");
          }

          // Try to get cached JWT
          const cachedJwt = loadJwt();
          if (!cachedJwt) {
            throw new Error("Failed to load token.");
          }

          handleJwt(cachedJwt, server);
          setState({ loading: false, error: null });
          return;
        }

        const json = await response.json();
        const jwt = json.jwt || json.token_value;
        if (!jwt) {
          throw new Error("Failed to load token.");
        }

          handleJwt(jwt, server);
          saveJwt(jwt);
          setState({ loading: false, error: null });
      } catch (error) {
        const message = error instanceof Error ? error.message : "Failed to load token.";
        setState({ loading: false, error: message });
      }
    };

    fetchToken();
  }, [ltiServer]);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/components/tokenRetriever/LtiTokenRetriever.test.jsx Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@sonarqubecloud

Copy link
Copy Markdown

@sebastianchristopher sebastianchristopher left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍👍👍

@buckett
buckett merged commit a152e2d into master Jan 30, 2026
6 checks passed
@buckett
buckett deleted the double-fetch-prevent branch January 30, 2026 14:52
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.

3 participants