-
Notifications
You must be signed in to change notification settings - Fork 378
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
Bugfix/live 5056 error mapping not found entity #5315
Bugfix/live 5056 error mapping not found entity #5315
Conversation
🦋 Changeset detectedLatest commit: 6da823c The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
4 Ignored Deployments
|
@@ -457,9 +465,9 @@ const getDeviceVersion: (targetId: string | number, provider: number) => Promise | |||
target_id: targetId, | |||
}, | |||
}).catch(error => { | |||
const status = error && (error.status || (error.response && error.response.status)); // FIXME LLD is doing error remapping already. we probably need to move the remapping in live-common | |||
const status = error?.status || error?.response?.status; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah cleaner :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🦄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice fix LGTM
7091bfd
to
6da823c
Compare
* fix(common, llm, lld): better wording of not found entity error * chore: changeset * fix(common/manager api): remap 404 error in get_firmware_version * fix(lld, llm): wording capitalisation * fix(llm/pairing/RenderError): properly render wrong provider error * refactor(common/manager/api): clearer code + remove 4 year old FIXME * fix(common/manager/api): remove bad practice of checking error message
* fix(common, llm, lld): better wording of not found entity error * chore: changeset * fix(common/manager api): remap 404 error in get_firmware_version * fix(lld, llm): wording capitalisation * fix(llm/pairing/RenderError): properly render wrong provider error * refactor(common/manager/api): clearer code + remove 4 year old FIXME * fix(common/manager/api): remove bad practice of checking error message
📝 Description
Improve the error mapping for the 404 response from some endpoints of the manager API, which is due to the provider number being wrong for the requested firmware version.
Currently the message shown is either:
Device firmware is not recognized, please contact Ledger support
on LLD if the error comes from the/get_device_version
API call.not found entity
.The new improved message is:
Invalid Provider
You have to change "My Ledger provider" setting. To change it, open Ledger Live "Settings", select "Experimental features", and then select a different provider.
❓ Context
common
lld
llm
✅ Checklist
📸 Demo
LLD
Screen.Recording.2023-11-03.at.17.25.56.mov
LLM
Simulator.Screen.Recording.-.iPhone.14.-.2023-11-03.at.17.06.38.mp4
LLM old device pairing flow
RPReplay_Final1699269969.MP4
🚀 Expectations to reach
Please make sure you follow these Important Steps.
Pull Requests must pass the CI and be internally validated in order to be merged.