Skip to content

Conversation

bitfield
Copy link
Owner

@bitfield bitfield commented Nov 5, 2022

No description provided.

bitfield and others added 3 commits November 5, 2022 16:15
Co-authored-by: Christoph Petrausch <263448+hikhvar@users.noreply.github.com>
Co-authored-by: James Mills <prologic@shortcircuit.net.au>
Co-authored-by: Carl Johnson <me@carlmjohnson.net>
@bitfield
Copy link
Owner Author

bitfield commented Nov 8, 2022

@hikhvar @carlmjohnson @M3talM0nk3y @lalloni maybe you'd be interested in trying out this branch, as you've all had valuable input into the design. Does the Do/Get/Post API support the kind of programs you want to write involving HTTP requests? Can you find any bugs or unexpected behaviour? And is there anything missing which you need for real-world use cases?

@M3talM0nk3y
Copy link

@bitfield Looks like it is taking shape. Have you thought about how to allow the user to configure underlying http client as well as provide custom headers for things like auth (basic and bearer, for example)?

In the case of a POST/PUT/PATCH, is the idea to allow the body be provided from the standard input and/or a file?

@bitfield
Copy link
Owner Author

bitfield commented Nov 9, 2022

Thanks @M3talM0nk3y! If you need to customise the request in some way (for example headers), use http.NewRequest as normal and then pass the req to script.Do. This is in the docs, but I'll add an example to make it clearer, thank you for pointing that out.

Yes, Get, Post, and Do all use the contents of the pipe as the request body. I'd be very pleased if you could test this out for me with a couple of different kinds of request, and make sure it works as advertised.

@bitfield
Copy link
Owner Author

One good question raised by @thiagonache is "what if you want the status code?" Right now, any non-2xx response sets an error and makes the pipe a no-op. But there are non-2xx status codes that aren't errors; for example, 304 Not Modified. What should we do in that case?

@earthboundkid
Copy link
Contributor

Go automatically follows redirects. You only get a 304 if you set an ETAG/If-Not-Modifier header, in which case, you probably don't need the body anyway. So, it's probably mostly okay except if you want to log an error. Can the body be sent to stderr if it's a non-2XX response?

@thiagonache
Copy link
Contributor

thiagonache commented Nov 10, 2022

Go automatically follows redirects. You only get a 304 if you set an ETAG/If-Not-Modifier header, in which case, you probably don't need the body anyway. So, it's probably mostly okay except if you want to log an error. Can the body be sent to stderr if it's a non-2XX response?

@bitfield and I agreed that in most cases it's not a problem. I think the idea here is to check if someone has any valid case where a non-2XX response would be useful

@hikhvar
Copy link
Contributor

hikhvar commented Nov 10, 2022

I would vote to leave it. If somebody really needs that special behavior, they could either implement an own Source returning a new pipe, or implementing a proper Filter.

In other libraries I would make an own package for the detailed and customisable implementation of the source/filters. But as far as I understood this is against the philosophies of this library.

@bitfield
Copy link
Owner Author

Can the body be sent to stderr if it's a non-2XX response?

This is a nice idea for the general problem of "if there's any error, the program just prints nothing". Maybe Stdout should print the error message on stderr, if there is one—or perhaps this should be a new method, in case anyone is relying on the old behaviour.

@bitfield
Copy link
Owner Author

What about an Exit method, that prints any output to stdout, any errors to stderr, and sets the os.Exit status accordingly? Then you could write programs like:

script.Get(URL).Exit()

and they might produce:

HTTP/1.1 404 Not Found

with

echo $?
1

@bitfield bitfield merged commit ad52e8c into master Nov 11, 2022
@bitfield bitfield deleted the http branch November 11, 2022 18:55
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.

5 participants