Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http.MethodGet lost setLevel #1225

Open
lxb325710 opened this issue Jan 30, 2023 · 2 comments
Open

http.MethodGet lost setLevel #1225

lxb325710 opened this issue Jan 30, 2023 · 2 comments

Comments

@lxb325710
Copy link

func (lvl AtomicLevel) ServeHTTP(w http.ResponseWriter, r *http.Request) {
type errorResponse struct {
Error string json:"error"
}
type payload struct {
Level zapcore.Level json:"level"
}

enc := json.NewEncoder(w)

switch r.Method {
case http.MethodGet:
        lvl.SetLevel(requestedLvl)  //Bug:lost  lvl.SetLevel(requestedLvl)
	enc.Encode(payload{Level: lvl.Level()})
case http.MethodPut:
	requestedLvl, err := decodePutRequest(r.Header.Get("Content-Type"), r)
	if err != nil {
		w.WriteHeader(http.StatusBadRequest)
		enc.Encode(errorResponse{Error: err.Error()})
		return
	}
	lvl.SetLevel(requestedLvl)
	enc.Encode(payload{Level: lvl.Level()})
default:
	w.WriteHeader(http.StatusMethodNotAllowed)
	enc.Encode(errorResponse{
		Error: "Only GET and PUT are supported.",
	})
}

}

@abhinav
Copy link
Collaborator

abhinav commented Feb 1, 2023

@lxb325710 Can you clarify?

Get should not call setLevel. That should be a read-only operation.

@ozfive
Copy link

ozfive commented Mar 11, 2023

I agree with @abhinav's comment. The Get operation should not call SetLevel. The Get method should only retrieve the current atomic level and return it as the response payload. The SetLevel operation should only be triggered by a Put request, where the requested level is decoded and passed to the SetLevel function.

In case http.MethodGet: you should just be getting the current level by setting it like so currentLvl := lvl.Level(). The code below is what you want:

func (lvl AtomicLevel) ServeHTTP(w http.ResponseWriter, r *http.Request) {
	type errorResponse struct {
		Error string `json:"error"`
	}
	type payload struct {
		Level zapcore.Level `json:"level"`
	}

	enc := json.NewEncoder(w)

	switch r.Method {
	case http.MethodGet:
		currentLvl := lvl.Level()
		enc.Encode(payload{Level: currentLvl})
	case http.MethodPut:
		requestedLvl, err := decodePutRequest(r.Header.Get("Content-Type"), r)
		if err != nil {
			w.WriteHeader(http.StatusBadRequest)
			enc.Encode(errorResponse{Error: err.Error()})
			return
		}
		lvl.SetLevel(requestedLvl)
		enc.Encode(payload{Level: lvl.Level()})
	default:
		w.WriteHeader(http.StatusMethodNotAllowed)
		enc.Encode(errorResponse{
			Error: "Only GET and PUT are supported.",
		})
	}
}

func decodePutRequest(contentType string, r *http.Request) (zapcore.Level, error) {
	if contentType != "application/json" {
		return zap.InfoLevel, fmt.Errorf("Content-Type must be application/json")
	}
	var payload struct {
		Level string `json:"level"`
	}
	if err := json.NewDecoder(r.Body).Decode(&payload); err != nil {
		return zap.InfoLevel, err
	}
	level, err := zapcore.ParseLevel(payload.Level)
	if err != nil {
		return zap.InfoLevel, err
	}
	return level, nil
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants