Add's more descriptive error messages for 429 errors and other errors with no body #1082
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
ericanderson
requested changes
Jan 9, 2025
| ) { | ||
| if (response?.status && response.statusText) { | ||
| // Catches errors like 429 where there is no response body but a code is returned | ||
| return new UnknownError( |
There was a problem hiding this comment.
Hmm. I don't think a statusText is a requirement. I'd probably not do this separate if statement and instead just include the statusCode on the 2 throws below. If you think the status text is important you could add it to the first argument of the message down there.
The use of UNKNOWN aligns with the existing palantir api errors that are used by conjure and co and that populate through so using a custom message like the statusText for that feels off.
This reverts commit a89d71c.
nihalbhatnagar
commented
Jan 9, 2025
| throw convertError( | ||
| e, | ||
| "A network error occurred while reading response", | ||
| throw new PalantirApiError( |
There was a problem hiding this comment.
This is fine since we already know we're in an error state. The JSON body being unparseable isn't an error here
There was a problem hiding this comment.
Nice, all thats missing is actually passing the status code as the forth argument
ericanderson
requested changes
Jan 10, 2025
| throw convertError( | ||
| e, | ||
| "A network error occurred while reading response", | ||
| throw new PalantirApiError( |
There was a problem hiding this comment.
Nice, all thats missing is actually passing the status code as the forth argument
ericanderson
approved these changes
Jan 10, 2025
ericanderson
added a commit
that referenced
this pull request
Jan 13, 2025
* origin/main: Add typed lint checks and fix promise related things (#1081) Version Packages (beta) (#1090) Export hydrateObjectSetFromRid from internal (#1087) Fetch by RID experimental (#1088) Creates new internal export for helper methods we want to hide (#1084) Add's more descriptive error messages for 429 errors and other errors with no body (#1082) Update react template per discussion (#1077) OSDK client internal property is hidden from output .d.ts (#1079) Revert `api:x-read/write` back to `api:read/write-x` (#1076) Move from "views" naming to "widgets" (#1075) Fixes attachment uploads in Browser context (#1064) Use foundry-config-json in widget-manifest-vite-plugin (#1055) Version Packages (beta) (#1072) Ignore unknown types (#1067) Correct widget template package dependency versions (#1068) Make interface type status configurable (#1069) Fix onOutOfDate handler from firing twice (#1071) Rename @osdk/widget-manifest-vite-plugin to @osdk/widget.vite-plugin.unstable (#1065) Fixing main (#1066) Add structs read support (#1047) Handle Vite plugin injections in Custom Widget Vite plugin (#1063) Version Packages (beta) (#1060) Fix unused import (#1061) Revive eslint-plugin-unused-imports (#1057) Fix URL for getBulkLinks (#1058) Split foundry-config-json package (#1054) Add package.json auto version strategy (#1052) Remove crypto from OSDK (#1051) Support site snapshot upload (#1049) Try catch subscription handlers (#1019) Experiment with no proxy for objects (#1031) Bump unstable widget packages to v1 to avoid undesired v0.x version behavior (#1050) WeakRef the as()'d objects (#1043)
ericanderson
added a commit
that referenced
this pull request
Jan 13, 2025
…ated-permissions-cause-403-errors-when-user-loads-new-version-of-app-provide-way-to-get-new-token-with-new-permissions-on-load * origin/main: Fixes infinite redirect issue on some errors in the OAuth flow (#1089) Tighten return types (#924) Add typed lint checks and fix promise related things (#1081) Version Packages (beta) (#1090) Export hydrateObjectSetFromRid from internal (#1087) Fetch by RID experimental (#1088) Creates new internal export for helper methods we want to hide (#1084) Add's more descriptive error messages for 429 errors and other errors with no body (#1082) Update react template per discussion (#1077) OSDK client internal property is hidden from output .d.ts (#1079) Revert `api:x-read/write` back to `api:read/write-x` (#1076) Move from "views" naming to "widgets" (#1075)
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.
Fixes #1070