Provide example for SetExtraHTTPHeaders

[s3rj1k]

I was looking for example SetExtraHTTPHeaders for setting Accept-Language headers

[mafredri]

I wasn't sure at first why you needed an example of this, but I took a look at the input arguments to the function and noticed what might be the reason.

Headers are defined as:

// Headers Request / response headers as keys / values of JSON object.
type Headers []byte

It is kept as a byte slice (raw JSON) to make it more convenient to decode headers into any type of struct (instead of, say a map[string]string).

Maybe I could add a convenience method to create headers from a map as well, something like:

func HeadersFromMap(m map[string]string) (Headers, error) {
	b, err := json.Marshal(m)
	if err != nil {
		return nil, err
	}
	return b, nil
}

Thoughts?

[s3rj1k]

@mafredri would be nice, more readable that way.

only map needs to defined from outside of library

[mafredri]

only map needs to defined from outside of library

How do you mean?

[s3rj1k]

var headers map[string]string
...
HeadersFromMap(header)
...

[mafredri]

I'm not sure I follow, e.g. what's the problem? You could just as well do:

HeadersFromMap(map[string]string{"Accept-Language": "*"})

I was thinking of a potential parallel method (or alternative, haven't decided quite on what API additions to introduce)

const missingValue = "(MISSING)"

func (n Headers) FromKV(keyvals ...string) Headers {
	if len(keyvals) == 0 {
		return nil
	}
	if len(keyvals)%2 != 0 {
		keyvals = append(keyvals, missingValue)
	}
	m := make(map[string]string)
	for i := 0; i < len(keyvals); i = i + 2 {
		m[keyvals[i]] = keyvals[i+1]
	}
	b, err := json.Marshal(m)
	if err != nil {
		return []byte(err.Error())
	}
	return b
}

I'm not sure if this is an acceptable way to avoid returning an error. Returning an error would make this much less fluid, and I don't think encoding a map[string]string to JSON should ever return an error, so it would be unfortunate to return one.

If there ever was an error, the headers would become invalid JSON (via return []byte(err.Error())) and should give an error a bit further down the line instead. Will have to think about this a bit 🤔.

[s3rj1k]

You can return empty []byte on error

[mafredri]

You can return empty []byte on error

But this would not inform the user that encoding failed, and debugging why something isn't working (or headers missing) would be non-obvious.

[s3rj1k]

User expect that returned value is non empty, so if it is actually empty, this is error

although detailed err message would be hidden

[mafredri]

User expect that returned value is non empty, so if it is actually empty, this is error

Why would you say that is?

Let's say FromMap was fed an empty map? Surely nil is a valid return value then. Or let's say we invoke FromKV(...args), where args is an empty slice, again nil would be a very valid return value. Especially considering that in some cases we want Headers to be nil (when relying on omitempty).

In my opinion, a function that returns no error can also never fail (except maybe panic). Or at least, that is the common expectation for users.

[s3rj1k]

If it is a helper function, user will use it for marshalling supplied headers, so nil output would be an error from users point of view.

In this case user should validate input, that it is not empty ...slice or map, if it is not and func returns zero-sized variable this will tell that error accrued.

I agree on usage case of panic, but panic often complicates things.

Can we just return data and error?

[mafredri]

If it is a helper function, user will use it for marshalling supplied headers, so nil output would be an error from users point of view.

I don't trust that every user will see this the same way. Consider the following (given above implementation):

var keyvals []string
if shouldAddHeadersY {
    keyvals = append(keyvals, "My-AwesomeY", "Header")
}
if shouldAddHeadersX {
    keyvals = append(keyvals, "My-AwesomeX", "Header")
}
headers := network.HeadersFromKV(keyvals...)
c.Network.SetExtraHTTPHeaders(headers)

I'm struggling to see any use case where nil from network.HeadersFromKV should be an error, and this totally feels like code I might write.

I agree on usage case of panic, but panic often complicates things.

Absolutely, and this is why cdp barely has any panics. And I'm not keen on introducing any ;).

Can we just return data and error?

That's probably best, yeah :)

[s3rj1k]

Any time frame on this? I am eager to test this out :)

[mafredri]

Sorry, can't provide any guarantees. I'll try to get around to it soon, probably during next week at the latest.

[s3rj1k]

hi @mafredri, any news on that?

[mafredri]

Sorry, still haven’t gotten around to give this enough thought. I try to avoid making breaking changes which unfortunately also means I end up spending a lot of time thinking of solutions before pushing them out 😕.

[mafredri]

I'm struggling with finding a good purpose for adding this to the library. Considering the HeadersFromMap from above, it really is no different than replacing network.HeadersFromMap with json.Marshal in the code. Both require error checking, so it's doesn't even increase convenience.

The only reason to add it, that I can think of, is to:

• Aid with discoverability / purpose of the Headers type.

But IMO this could be accomplished by improving the code comment for type Headers.

What are your thoughts @s3rj1k?

[s3rj1k]

Hi @mafredri, you can just provide detailed example on how to do it and no changes will be required for library.

[mafredri]

I'm not sure I understand what type of example you're asking for?

You could use it like this:

err := c.Network.SetExtraHTTPHeaders(ctx,
	network.NewSetExtraHTTPHeadersArgs(
		[]byte(`{"Content-Type": "text/html; charset=utf-8"}`),
	),
)

But IMO this information is already available in this thread, is it something else you're looking for?

[s3rj1k]

@mafredri Hi, sorry for late reply

can't get working NewSetExtraHTTPHeadersArgs nor c.Emulation.SetUserAgentOverride
:(

[mafredri]

Can you post the code that you've tried to use? I'll try to have a look during this week.

[s3rj1k]

https://gist.github.com/s3rj1k/fa6d9b4da2a97666a6eb20d3e3fd077c

modified test case, output should include [HTTP_ACCEPT_LANGUAGE] header

[mafredri]

Have you tried enabling the Network domain? After adding c.Network.Enable(ctx, nil) (before c.Network.SetExtraHTTPHeaders), it works for me. Running Chrome Version 68.0.3440.59 (Official Build) beta.

[vjiandani-r7]

I'm having this same problem. I'm using Chrome Version 68.0.3440.106. I used your suggestion to eliminate any other possibilities. My code looks exactly like the following. I tested it with google.com and it creates the pdf without any issues but it doesn't work on a site that actually needs the header.

func CreatePdf(ctx context.Context, request GeneratePdfRequest) ([]byte, error) {

	//headers, marshallErr := json.Marshal(request.Headers)
	//if marshallErr != nil {
	//	return nil, marshallErr
	//}
	//extraHeaders := network.NewSetExtraHTTPHeadersArgs(headers)

	// Use the DevTools API to manage targets
	devt := devtool.New("http://127.0.0.1:9222")
	pt, _ := devt.Create(ctx)
	defer devt.Close(ctx, pt)

	// Open a new RPC connection to the Chrome Debugging Protocol target
	conn, _ := rpcc.DialContext(ctx, pt.WebSocketDebuggerURL)
	defer conn.Close()

	// Create new browser context
	baseBrowser := cdp.NewClient(conn)
	newContextTarget, _ := baseBrowser.Target.CreateBrowserContext(ctx)

	// Create a new blank target with the new browser context
	newTargetArgs := target.NewCreateTargetArgs("about:blank").
		SetBrowserContextID(newContextTarget.BrowserContextID)
	newTarget, _ := baseBrowser.Target.CreateTarget(ctx, newTargetArgs)

	// Connect the client to the new target
	newTargetWsURL := fmt.Sprintf("ws://127.0.0.1:9222/devtools/page/%s", newTarget.TargetID)
	newContextConn, _ := rpcc.DialContext(ctx, newTargetWsURL)
	defer newContextConn.Close()
	c := cdp.NewClient(newContextConn)

	// Close the target when done
	// (In development, skip this step to leave tabs open!)
	closeTargetArgs := target.NewCloseTargetArgs(newTarget.TargetID)
	defer baseBrowser.Target.CloseTarget(ctx, closeTargetArgs)

	// Enable the runtime
	err := c.Runtime.Enable(ctx)
	if err != nil {
		return nil, err;
	}

	// Enable the network
	_ = c.Network.Enable(ctx, network.NewEnableArgs())
	//_ = c.Network.SetExtraHTTPHeaders(ctx, extraHeaders)
	c.Network.SetExtraHTTPHeaders(ctx,
		network.NewSetExtraHTTPHeadersArgs(
			[]byte(`{"CDN_BUCKET": "som_url"`),
		),
	)

	// Enable events on the Page domain
	_ = c.Page.Enable(ctx)

	// Create a client to listen for the load event to be fired
	loadEventFiredClient, _ := c.Page.LoadEventFired(ctx)
	defer loadEventFiredClient.Close()

	// Tell the page to navigate to the URL
	navArgs := page.NewNavigateArgs(request.TargetUrl)
	c.Page.Navigate(ctx, navArgs)

	// Wait for the page to finish loading
	_, _ = loadEventFiredClient.Recv()

	// Print to PDF
	printToPDFArgs := page.NewPrintToPDFArgs().
		SetLandscape(request.Orientation == "Landscape").
		SetPrintBackground(true).
		SetMarginTop(0).
		SetMarginBottom(0).
		SetMarginLeft(0).
		SetMarginRight(0).
		SetPrintBackground(true)
	pdf, _ := c.Page.PrintToPDF(ctx, printToPDFArgs)

	return pdf.Data, nil
}

[mafredri]

@vjiandani-r7 I really do recommend checking for errors, for example, the JSON you are using in network.NewSetExtraHTTPHeadersArgs is invalid. I believe c.Network.SetExtraHTTPHeaders should tell you this as well when checking the error.

Hope that fixes it!

[vjiandani-r7]

I added in error handling and confirmed that I fixed the error Error: cdp.Network: SetExtraHTTPHeaders: json: error calling MarshalJSON for type network.Headers: unexpected end of JSON input.

It still doesn't work.

[mafredri]

@vjiandani-r7 how did you confirm that it doesn't work? Have you looked at the headers sent to the webserver?

Here's a gist of what I've used to test this and there are no issues setting / sending the headers.

[vjiandani-r7]

@mafredri Hmm I confirmed with your gist that the headers are being passed along correctly but I'm still having issues. I'll have to do some more debugging to figure out why the page I'm trying to load thinks they're missing. Thanks for your help.

[mafredri]

@vjiandani-r7 alright, I hope you figure it out!

@s3rj1k I hope this issue was resolved for you, feel free to re-open if there's anything left to discuss.