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

Bgs 153/nik second refinement #2359

Closed
wants to merge 18 commits into from
Closed

Conversation

monkpow
Copy link
Collaborator

@monkpow monkpow commented Jan 17, 2025

No description provided.

@svc-apollo-docs
Copy link
Collaborator

svc-apollo-docs commented Jan 17, 2025

✅ Docs preview has no changes

The preview was not built because there were no changes.

Build ID: b555431a8a64d8b04f621147

Comment on lines +98 to 113
//})
.map_err(|err| {
match err {
GraphQLServiceError::InvalidCredentials() => {
RoverClientError::PermissionError {
msg: "attempting to fetch subgraphs".to_string(),
}
}
_ => {
RoverClientError::Service {
source: Box::new(err),
endpoint_kind: EndpointKind::ApolloStudio,
}
}
}
})
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this came out all right. This maps the GraphQLServiceError to a RoverClientError of the correct flavor. I didn't cause this PermissionError to track source, which seems like a good idea to implement (but later).

Comment on lines +131 to +136
match RoverClientError::from(err) {
RoverClientError::PermissionError { msg } => {
LoadRemoteSubgraphsError::FetchRemoteSubgraphsAuthError(Box::new(err))
},
_ => { LoadRemoteSubgraphsError::FetchRemoteSubgraphsError(Box::new(err)) },
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm back in the same dang hole here. Even though this appears to me to literally be a RoverClientError (as that is the response type from the adjusted method in FetchAllSubgraphsService), here we don't know that, and I need some way to negotiate the type conversion. I banged my shins on writing a From in the error file, but didn't get anything working over there.

Appreciate any feedback or pointers, even if it's "This is all wrong" :)

Comment on lines +355 to +359
//the trait bound `rover_client::RoverClientError: From<<S as MakeService<(), FetchRemoteSubgraphsRequest>>::Error>` is not satisfied
//the trait `From<<S as MakeService<(), FetchRemoteSubgraphsRequest>>::Error>` is not implemented for `rover_client::RoverClientError`
//impl From<<S as MakeService<(), FetchRemoteSubgraphsRequest>>::Error> for RoverClientError {
//}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This commented section is referenced below; a rough start to attempting to impl a From trait for the pattern match I'm working on.

@monkpow monkpow closed this Jan 17, 2025
@monkpow monkpow deleted the BGS-153/nik-second-refinement branch January 17, 2025 22:53
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