Skip to content

Rescue Http errors with given message - #844

Merged
Polo2 merged 1 commit into
mainfrom
add-default--error-code-support
Oct 15, 2026
Merged

Polo2 merged 1 commit into
mainfrom
add-default--error-code-support

Conversation

@Polo2

@Polo2 Polo2 commented Oct 13, 2026

Copy link
Copy Markdown
Member

4xx only, read http error message

This is a long term missing feature in the CLI
(noticed while implementing paywall and error payment_required with status 402 on Bump.sh API).

With this commit, when http error code is 4xx and not explicitly catched (400, 401, 404, 422),
we return the provided error message to the CLI user, with the status.

@Polo2 Polo2 self-assigned this Oct 13, 2026
Comment thread src/api/error.ts Outdated
Comment thread src/api/error.ts Outdated
@Polo2

Polo2 commented Oct 14, 2026

Copy link
Copy Markdown
Member Author

Thanks dear review,
following your comments and a little more reflexion far from my keyboard, I rewrote the PR to something easier to review and merge:

There is already the info.length > 0 validation,
and generic message is raised when info is nil:
Unhandled API error (status: ${status}) (error: ${httpError}),
even if the error response had a message -> that's the case I want to fix in this PR!

In this PR, we add a default behavior (outside 400, 401, 404 and 422 explicit error codes),
to reuse the provde error message if it's possible.

Else, fallback to current behavior.

I pushed this in 2nd commit, WDYT?

@Polo2 Polo2 changed the title Error 4xx, generic message Rescue errors with given message Oct 14, 2026
@Polo2 Polo2 changed the title Rescue errors with given message Rescue Http errors with given message Oct 14, 2026
@Polo2
Polo2 requested a review from fbraure October 15, 2026 07:26
Comment thread src/api/error.ts Outdated
@Polo2
Polo2 force-pushed the add-default--error-code-support branch from bb739b1 to c626a30 Compare October 15, 2026 09:39

@fbraure fbraure left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@Polo2

Polo2 commented Oct 15, 2026

Copy link
Copy Markdown
Member Author

Thanks for the ✅ , I'll squash commits and fix the conflict before merging it

This is a long term missing feature in the CLI
(noticed while implementing paywall and error :payment_required
on Bump.sh API).

With this commit, when http error code has a message and not explicitly catched
(400, 401, 404, 422),
we display the provided error message to the CLI user, with the status.

There is already the `info.length > 0` validation,
and generic message is raised when info is nil:
`Unhandled API error (status: ${status}) (error: ${httpError})`

Else, fallback to current behavior:
> `Unhandled API error (status: ${status}) (error: ${httpError})`
@Polo2
Polo2 force-pushed the add-default--error-code-support branch from c626a30 to 59bf8cb Compare October 15, 2026 10:10
@Polo2
Polo2 merged commit 4307bb5 into main Oct 15, 2026
9 checks passed
Comment thread src/api/error.ts
Comment on lines +61 to +63
static genericErrorFromMessage(error: Error): MessagesAndExitCode {
return error.message ? [[error.message], 100] : [[], 100]
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would have kept the http status to have the same logic as the other errors :)

Suggested change
static genericErrorFromMessage(error: Error): MessagesAndExitCode {
return error.message ? [[error.message], 100] : [[], 100]
}
static genericErrorFromMessage(error: Error, status: number): MessagesAndExitCode {
return [error.message ? [error.message] : [], status - 300]
}

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