Adapt CLI to receive 402 status from Bump API #465
Conversation
src/api/error.ts
Outdated
| const genericMessage = | ||
| error.message || 'You need to upgrade to a paid plan to perform this request.'; | ||
|
|
||
| return [[genericMessage], 104]; |
There was a problem hiding this comment.
I had a convention to substract 300 to the http code status (404-300 = 104 CLI error, 422-300 = 122 CLI error), so for a 402 HTTP status I'd suggest a 102 CLI error.
There's no particular reason, nor suggestion for the rest of the world about this. I decided this convention by myself for “fun”. You can totally ignore this but if you make the change it would keep the exit codes coherent together (but completely incoherent to the rest of the world 😛)
There was a problem hiding this comment.
Oh great, if there is a convention only known by Paul developers, I'd be glad to follow it :)
I'll adapt code if we finally decide to return response 402 instead of 400 in API.
There was a problem hiding this comment.
Following https://github.com/bump-sh/bump/issues/3302#issuecomment-1557174013, Bump API will return response with error status 402 when payment is required by resource owner.
Thus, I've adapted this PR with 102 CLI error, to stick with the very secret convention here 😛
We are currently discussing to add a specific response to our Bump API, when resource (Api or Hub) is disabled (which mean that resource owner has to upgrade its plan): bump-sh/bump#3326 A possibility is to use HTTP status code 402 'Payment required': https://developer.mozilla.org/fr/docs/Web/HTTP/Status/402 This PR adapt CLI to support HTTP status 402, and raise error 102 with provided message (or default message defined in CLI).
Polo2
force-pushed
the
Adapt-CLI-to-support-error-402
branch
from
6ef68a7 to
0323524
Compare
|
Last question here, what will happens on CLI with version 2.5.0 or earlier? If I'm not wrong here, and if we approve this PR, support of error 402 will be included in next release. But what currently happens when error code in not supported? |
That's my question: https://github.com/bump-sh/bump/issues/3302#issuecomment-1559635441 😅 |
♻️ Excel error, circular reference 😅 As discussed in product meeting today, for this reason, let's favor error 400 that is already fully supported (and improve CLI with this issue #466 to support more error codes)) |
|
@Polo2 will you adapt this PR to support “any error code” (instead of just 402) or do you prefer to close it and re-open a new one? |
I don't know if I will handle the "any error code" personally (cf issue #466 ), but anyway I think this should be handled in a dedicated PR, thus we can close this one. |
We are currently discussing to add a specific response to our Bump API, when resource (Api or Hub) is disabled
(which mean that resource owner has to upgrade its plan): https://github.com/bump-sh/bump/pull/3326
A possibility is to use HTTP status code 402 'Payment required': https://developer.mozilla.org/fr/docs/Web/HTTP/Status/402
With this PR, I tried to adapt our CLI to support such error code 402.
It's really open to discussion since it's almost my first 'true' PR on this repository.