Skip to content

refactor: renew API client (request & response abstraction by struct)#63

Merged
ninoseki merged 3 commits intomainfrom
renew-api
Aug 12, 2025
Merged

refactor: renew API client (request & response abstraction by struct)#63
ninoseki merged 3 commits intomainfrom
renew-api

Conversation

@ninoseki
Copy link
Collaborator

@ninoseki ninoseki commented Aug 8, 2025

A bit (or very) opinionated PR to refactor the API module.

I think having structs for handling request & response is better since it's more open to extend.

For example,

Before

func (cli *Client) parseResponse(resp *http.Response) (*JSONResponse, error) {
	jsonResp := &JSONResponse{}

	if !strings.HasPrefix(resp.Header.Get("Content-Type"), "application/json") {
		return nil, fmt.Errorf("expecting JSON response from %s %s",
			resp.Request.Method, resp.Request.URL.String())
	}

	var reader = resp.Body
	read, err := io.ReadAll(reader)
	if err != nil {
		return nil, err
	}
	// consider 2xx response as successful
	if resp.StatusCode >= 200 && resp.StatusCode < 300 {
		jsonResp.Raw = json.RawMessage(read)
		return jsonResp, nil
	}

	jsonErr := &JSONError{}
	err = json.Unmarshal(read, jsonErr)
	if err != nil {
		return nil, err
	}
	return nil, jsonErr
}

...

func (cli *Client) DoWithJSONParse(req *http.Request) (*JSONResponse, error) {
	resp, err := cli.httpClient.Do(req)
	if err != nil {
		return nil, err
	}
	defer resp.Body.Close() //nolint:errcheck

	return cli.parseResponse(resp)
}

(It needs a parsing method (func parse...) & customized Do method (func DoWith...) per format)

After

func (r *Response) ToJSON() (*json.RawMessage, error) {
	body, err := r.ToBytes()
	if err != nil {
		return nil, err
	}

	contentType := r.GetContentType()
	if !strings.Contains(contentType, "application/json") {
		return nil, fmt.Errorf("response is not JSON, content-type: %s", contentType)
	}
	raw := json.RawMessage(body)
	return &raw, nil
}

...

resp, err := client.NewRequest().Get("/")
if err != nil {
	return err
}
resp.ToJSON()

(It only needs a response's method per format)

I think the later interface is more easy to add a support of a new format, etc.

@ninoseki ninoseki changed the title refactor: renew API client to do method chaining refactor: renew API client (request & response abstraction by struct) Aug 8, 2025
@ninoseki ninoseki requested review from cdnsyseng and fw42 August 8, 2025 23:26
@cdnsyseng
Copy link
Contributor

@ninoseki looks fine, please look at #69 before merging this

Signed-off-by: Hal Martin <hal@urlscan.io>
@ninoseki ninoseki merged commit d0cb729 into main Aug 12, 2025
5 checks passed
@ninoseki ninoseki deleted the renew-api branch August 12, 2025 08:49
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.

2 participants