feat(Coder plugin): expose Coder SDK to Backstage end-users #132

Parkreiner · 2024-05-28T21:55:33Z

Part of #112

Changes made

Created useCoderQuery hook that simplifies wiring up all the Coder plugin logic to useQuery
Added an initial set of tests for the hook
Gave users the ability to disable the auth fallback UI altogether
Updated some of the test helpers

Example usage

This is from the end-user's perspective (useEndUserCoderAuth will already have been renamed to just useCoderAuth)

// Without useCoderQuery
const { sdk } = useCoderSdk();
const { isAuthenticated } = useCoderAuth();
const query = useQuery({
  queryKey: [CODER_QUERY_KEY_PREFIX, "workspaces"],
  queryFn: () => sdk.getWorkspaces({ q: "owner:me" }),
  enabled: isAuthenticated,
});

// This is fully equivalent and has additional properties get updated based on auth status
const query2 = useCoderQuery({
  queryKey: ["workspaces"],
  queryFn: ({ sdk }) => sdk.getWorkspaces({ q: "owner:me" })
});

Notes

I originally wanted to create a useCoderMutation hook, too, but decided to put it on the back-burner for now because:
- I fully expect queries to be the way that users interact with the API +75% of the time
- On Thursday, I'm going to start working on the Windows RDP module, and I knew I wouldn't have time to get both hooks ready and tested (need to test them individually and together)
- We still need some idea of what flows devs are going to be using with these hooks, so best not to commit to some false assumptions until we have more concrete user data
Will publish a new PR with documentation changes soon

code-asher

Nice! One thing I wonder about, from a user's perspective, is that the connection between the sdk and auth is not immediately obvious. Like, coming from other rest sdks, I think I would expect something like this: scratch that, see below

const sdk = makeCoderSdk(url, "token")
sdk.getWorkspaces()

Or to use Coder's auth provider:

const { token } = useCoderAuth() // Triggers the auth popup if no token yet
const sdk = makeCoderSdk(url, token)
sdk.getWorkspaces()

code-asher · 2024-05-31T20:59:23Z

plugins/backstage-plugin-coder/src/components/CoderProvider/CoderAuthProvider.tsx

        BackstageHttpError.isInstance(queryError) &&
        queryError.status === 401;

      if (!shouldRevalidate) {
        return;
      }

-      isRevalidatingToken = true;
+      isRevalidating = true;
      await queryClient.refetchQueries({ queryKey: sharedAuthQueryKey });


Not related to this PR, just a question because I think I might have skipped over this before or I just forgot.

This is not making the fetches again on a 401, right? That would just result in another 401. Is it to clear out the data or something?

It's sort of both? The logic spies on any 401s made for the currently-loaded query client, regardless of whether the 401 was for a Coder query or not.

Since we let the user supply their own Query client, the thinking was that even if a 401 triggers on a non-Coder query, it might help to revalidate the token, to handle cases like bad internet connections

One of the limitations of storing the auth validity state with React Query is that normally, the current state is tied to one specific API function, with one specific query key

So React Query doesn't automatically know that when a non-auth query function errors out, that the underlying auth is probably invalid, too.

Though now that I think about it, I should be able to update the auth state directly when a Coder query fails. Let me see if I can't get that fixed really quick

Think I have this improved now!
Revisiting the logic also made me realize that we don't really need the custom BackstageHttpError type anymore, now that we have Axios (it exposes a lot of the same properties)

Actually, never mind. Some of the changes are causing the existing tests to fail.
I'll open a new issue about this, but just to keep the scope of this PR small, I went ahead and reverted the changes

handle cases like bad internet connections

I think they would get some network error rather than a 401. Getting a 401 should mean the server responded and the token is well and truly invalid, and will never be valid again (at least if the token generating system is done correctly hahaha).

Although, it is interesting to consider the case where maybe they have a proxy in front of Coder, and it is the proxy that returned a 401 and not Coder, and the token is actually still valid. Buuuuut I guess even in that case the second query will still always fail with a 401, since a 401 is not automatically recoverable.

regardless of whether the 401 was for a Coder query or not

Ohhhhhhh I see, they could use it to make a request to some other service, and that service might give a 401? That does sound tricky! I see the use of the revalidate for this case but I agree it would be cool if it could be scoped to Coder queries.

plugins/backstage-plugin-coder/src/components/CoderProvider/CoderAuthProvider.tsx

plugins/backstage-plugin-coder/src/plugin.ts

code-asher · 2024-05-31T21:17:22Z

plugins/backstage-plugin-coder/src/hooks/reactQueryWrappers.ts

+ * @file Defines a couple of wrappers over React Query/Tanstack Query that make
+ * it easier to use the Coder SDK within UI logic.


Do most Backstage users use React Query? I worry we are abstracting too early.

If we just expose the base useCoderSdk and let users hook it up however they need, that seems most ideal to me.

I'm sure some of them do, but Backstage's default recommendations don't include it

I cover this in the README PR, but there are a lot of limitations around useEffect/useAsync, so I figured it would help to provide an easier way to bring RQ into the app. The plugin is still going to expose the SDK directly for people who want to wire things up in a more custom way

code-asher · 2024-05-31T21:28:30Z

Oh yeah wait the whole reason is to make it Backstage-y with the useApi stuff. Yeah gotta be the way you have it, I think.

Parkreiner · 2024-06-03T15:40:42Z

@code-asher Yeah, I tried to improve the co-location for the auth/SDK stuff to make their relationship a little more obvious, but when I tried it, I think the changes made things more confusing. I tried adding some auth-based helpers to useCoderSdk, but then it felt like that was blurring some of the lines for the responsibilities for it and useCoderAuth

I agree that the SDK setup flow you posted would be nice, but I don't know how to make that play nicely with React and Backstage. We don't have to do things the Backstage way – we could make it so that the user has to initialize the Coder Client themselves (making sure it's initialized outside a render function), and then feed that to the rest of the app via React Context. That's not the Backstage way, but it is the React way, which is good enough for me

The main thing I'm not 100% sure on is that, with the client being dependent on other Backstage APIs, you might have to jump through a ton of hoops to get feed those into the Coder Client since you lose access to API factories. It should be doable, but I'm not 100% sure if that might add more confusion, or might make the code even more complex

code-asher

Yeah, I think useApi like you have it now is the right abstraction to use for this.

Thinking out loud, maybe the relationship would be clearer if we had something like:

        return new CoderClient({
          apis: { urlSync, identityApi, coderAuthApi },
        });

And then CoderClient calls coderAuthApi.token() or something.

It feels a bit more Backstage-y to me over the React-ish way we have it now with the auth provider context. But idk if it is worth thinking about right now. Maybe when we revisit with oauth, since that will be a Backstage API as well. Could be cool if you could decide whether to slot that in or the current version with the pasting.

I do still wonder about the additional React Query abstraction, but going to defer to you since you know first-hand the pain users will experience doing it themselves. 😱

Parkreiner · 2024-06-04T14:53:43Z

@code-asher Yeah, this is definitely one area where I wish we had more active users to get feedback from. I definitely don't expect this to be the final design, but it feels like exposing the Coder API at all is going to a good way to entice people to try out the plugin, and get the feedback on how to make this better

I just wish that, for a platform built with React, Backstage were integrated with it a little bit better. The fact that we have to weigh the tradeoffs between React-isms and Backstage-isms doesn't feel super great

code-asher · 2024-06-04T17:41:11Z

I just wish that, for a platform built with React, Backstage were integrated with it a little bit better. The fact that we have to weigh the tradeoffs between React-isms and Backstage-isms doesn't feel super great

This 100%, could not agree more!!

Parkreiner added 18 commits May 24, 2024 14:58

chore: add vendored version of experimental Coder SDK

a27e95e

chore: update CoderClient class to use new SDK

9c958d3

chore: delete mock SDK

4979067

fix: improve data hiding for CoderSdk

5e7e01f

docs: update typo

937f6f5

Merge branch 'mes/vendored-sdk' into mes/vendored-sdk-integration

7e84d00

wip: commit progress on updating Coder client

294572d

wip: commit more progress on updating types

d9626a0

chore: remove valibot type definitions from global constants file

1dcc13b

chore: rename mocks file

692a763

fix: update type mismatches

28accc8

wip: commit more update progress

d032768

wip: commit progress on updating client/SDK integration

a76db16

fix: get all tests passing for CoderClient

d22bc20

fix: update UrlSync updates

08cd049

fix: get all tests passing

2eb4987

chore: update all mock data to use Coder core entity mocks

37645f4

fix: add extra helpers to useCoderSdk

864357d

Parkreiner self-assigned this May 28, 2024

Parkreiner marked this pull request as ready for review May 30, 2024 23:54

Parkreiner changed the title ~~feat(Coder plugin): improve developer ergonomics of using Coder SDK~~ feat(Coder plugin): expose Coder SDK to Backstage end-users May 31, 2024

Parkreiner added 4 commits May 31, 2024 15:30

fix: add additional properties to hide from SDK

977b2eb

Merge branch 'mes/vendored-sdk' into mes/vendored-sdk-integration

a9b24aa

Merge branch 'main' into mes/vendored-sdk-integration

259702e

fix: shrink down the API of useCoderSdk

09240cc

Parkreiner requested a review from code-asher May 31, 2024 17:21

code-asher reviewed May 31, 2024

View reviewed changes

Parkreiner added 2 commits June 3, 2024 13:39

update method name for clarity

3a8accb

chore: removal vestigal endpoint properties

a67fbcf

Base automatically changed from mes/vendored-sdk-integration to main June 3, 2024 14:23

Parkreiner force-pushed the mes/vendored-sdk-helpers branch from eabccd5 to a67fbcf Compare June 3, 2024 15:57

Merge branch 'main' into mes/vendored-sdk-helpers

c3c847a

Parkreiner mentioned this pull request Jun 3, 2024

Coder plugin: clean up error-handling logic #134

Open

fix: update reversion

6ef87b7

code-asher approved these changes Jun 3, 2024

View reviewed changes

Parkreiner merged commit 251214e into main Jun 4, 2024
2 checks passed

Parkreiner deleted the mes/vendored-sdk-helpers branch June 4, 2024 14:55

Parkreiner restored the mes/vendored-sdk-helpers branch June 11, 2024 03:36

Parkreiner deleted the mes/vendored-sdk-helpers branch June 17, 2024 19:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Coder plugin): expose Coder SDK to Backstage end-users #132

feat(Coder plugin): expose Coder SDK to Backstage end-users #132

Parkreiner commented May 28, 2024 •

edited

Loading

code-asher left a comment •

edited

Loading

code-asher May 31, 2024

Parkreiner Jun 3, 2024

Parkreiner Jun 3, 2024

Parkreiner Jun 3, 2024 •

edited

Loading

Parkreiner Jun 3, 2024

code-asher Jun 3, 2024

code-asher May 31, 2024

Parkreiner Jun 3, 2024

code-asher commented May 31, 2024

Parkreiner commented Jun 3, 2024

code-asher left a comment •

edited

Loading

Parkreiner commented Jun 4, 2024 •

edited

Loading

code-asher commented Jun 4, 2024

		* @file Defines a couple of wrappers over React Query/Tanstack Query that make
		* it easier to use the Coder SDK within UI logic.

feat(Coder plugin): expose Coder SDK to Backstage end-users #132

feat(Coder plugin): expose Coder SDK to Backstage end-users #132

Conversation

Parkreiner commented May 28, 2024 • edited Loading

Changes made

Example usage

Notes

code-asher left a comment • edited Loading

Choose a reason for hiding this comment

code-asher May 31, 2024

Choose a reason for hiding this comment

Parkreiner Jun 3, 2024

Choose a reason for hiding this comment

Parkreiner Jun 3, 2024

Choose a reason for hiding this comment

Parkreiner Jun 3, 2024 • edited Loading

Choose a reason for hiding this comment

Parkreiner Jun 3, 2024

Choose a reason for hiding this comment

code-asher Jun 3, 2024

Choose a reason for hiding this comment

code-asher May 31, 2024

Choose a reason for hiding this comment

Parkreiner Jun 3, 2024

Choose a reason for hiding this comment

code-asher commented May 31, 2024

Parkreiner commented Jun 3, 2024

code-asher left a comment • edited Loading

Choose a reason for hiding this comment

Parkreiner commented Jun 4, 2024 • edited Loading

code-asher commented Jun 4, 2024

Parkreiner commented May 28, 2024 •

edited

Loading

code-asher left a comment •

edited

Loading

Parkreiner Jun 3, 2024 •

edited

Loading

code-asher left a comment •

edited

Loading

Parkreiner commented Jun 4, 2024 •

edited

Loading