Update Members API error handling and Application Insights events #1462
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mec
force-pushed
the
fix/members-api-always-returns-result-object
branch
from
63f9267
to
2467ad2
Compare
mec
force-pushed
the
fix/members-api-always-returns-result-object
branch
from
2467ad2
to
fb657ae
Compare
lozette
approved these changes
Mar 27, 2024
This method returns two different objects, unlike all the other methods for this client. - When successful it returns the `MemberDetails` object - When unsuccessful it returns the`Result` object (like all other methods in the client) But this method is called from Project to return an MP and then we generally guard against a nil object. In the case where this fails the `project.member_for_constituency` is a `Result` object and not `nil` so things go wrong further along. The solution is to always return the `Result` object and then handle that in the calling code. Our first step is to return the Result with the `MemberDetails` and nil error. With that done, we handle the `Result` object in the calling code, allowing a `Project` to return `nil` for `member_of_parliament` when we cannot fetch on - which is what we want. We tweak a couple of specs to handle this change and move on.
We want to track some other events in Application Insights so we extract this out to a concern. At this point, it's a simple module but as a concern it has a conventional home.
The members API client should really return a result with either an error or the object that was fetched. Here we take the responsibility for tracking any errors in the API call out of the client and into the calling code, here in Project. This means will get insight on all the errors coming from the API calls and we only have one place to manage that. We've made the actual error message easier to identify in Application Insights by adding a prefix. We've updated some of the stubbed errors to return objects instead of their class, which is now required.
We want to surface when and why the Academies API is failing, like the Members API we can do this at the point of use in the Project.
mec
force-pushed
the
fix/members-api-always-returns-result-object
branch
from
fb657ae
to
a8996c0
Compare
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We are seeing some Members API errrors in production that are stopping some view and exports from completing, this work is mosly focused on resolving that.
We do so by making the Memeber API client always retrun the same
Resultobject and dealing with it appropriately.With that done, we update the Application Insights event tracking so we can better surface errors and add event tracking to the Academies API for good measure.