GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — feat: produce helpful error messages on httpRequest codec errors by bitgopatmcl · Pull Request #168 · BitGo/api-ts

Conversation

bitgopatmcl commented Jul 19, 2022

Copy link

Copy Markdown

Contributor

Attempts to produce a helpful error message when invalid codecs are passed to httpRequest.
It is a workaround inspired by this comment

Before:

After:

In this case the "before" error isn't too bad, but this technique could be useful in other places.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ericcrosson-bitgo reacted with heart emoji

All reactions

❤️ 1 reaction


          feat: produce helpful error messages on httpRequest codec errors

14343c6

Attempts to produce a helpful error message when invalid codecs are passed to `httpRequest`.
It is a workaround until something like microsoft/TypeScript#40468 is merged.

bitgopatmcl requested a review from a team

July 19, 2022 15:43

ghost reviewed

Jul 19, 2022

View reviewed changes

Comment thread

packages/io-ts-http/src/httpRequest.ts

		params?: EmitOutputTypeErrors<P['params'], string \| undefined, 'string \| undefined'>;
		query?: EmitOutputTypeErrors<
		P['query'],
		string \| string[] \| undefined,

ghost Jul 19, 2022

Copy link

Copy Markdown

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.

@bitgopatmcl is there already a type for ParamValue, QueryValue and HeaderValue? I was wondering why params and headers were a single string, before I realized it was actually the values' types. Using a type would make it more explicit, and hopefully the error output would remain readable.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bitgopatmcl reacted with thumbs up emoji

All reactions

👍 1 reaction

ericcrosson-bitgo approved these changes

Jul 19, 2022

View reviewed changes

ericcrosson-bitgo left a comment

Copy link

Copy Markdown

Contributor

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 improvement! I didn't know such a formatting was possible.

@louib-bitgo I'm not sure I follow, are you able to push a quick proof of concept to make sure we're on the same page?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

ericcrosson-bitgo merged commit 4673303 into BitGo:beta

Jul 19, 2022

github-actions Bot commented Jul 19, 2022

Copy link

Copy Markdown

🎉 This PR is included in version 0.2.0-beta.6 🎉

The release is available on npm package (@beta dist-tag)

Your semantic-release bot 📦🚀

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

github-actions Bot added the released on @beta label

Jul 19, 2022

github-actions Bot commented Jul 19, 2022

Copy link

Copy Markdown

🎉 This PR is included in version 1.0.0-beta.17 🎉

The release is available on npm package (@beta dist-tag)

Your semantic-release bot 📦🚀

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

github-actions Bot commented Jul 19, 2022

Copy link

Copy Markdown

🎉 This PR is included in version 0.2.0-beta.9 🎉

The release is available on npm package (@beta dist-tag)

Your semantic-release bot 📦🚀

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

github-actions Bot commented Jul 19, 2022

Copy link

Copy Markdown

🎉 This PR is included in version 0.2.0-beta.12 🎉

The release is available on npm package (@beta dist-tag)

Your semantic-release bot 📦🚀

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bitgopatmcl mentioned this pull request

Jul 19, 2022

chore: improve readability of query/param/header types #169

Merged

Reviewers

ericcrosson-bitgo

ericcrosson-bitgo approved these changes

+1 more reviewer

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

released on @beta

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

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.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.