From e3d8807cc75b275c63d6de3edad24a24e3662c36 Mon Sep 17 00:00:00 2001 From: ShiKhu Date: Mon, 18 Oct 2021 13:02:28 +0200 Subject: [PATCH] fix(backend-pg): no-rows to a project query response is an error --- backend/pkg/db/cache/messages_common.go | 49 +++++++++++++++++++++++++ backend/pkg/db/cache/messages_ios.go | 43 ---------------------- backend/pkg/db/cache/project.go | 4 +- backend/pkg/db/postgres/errors.go | 14 ++++--- backend/pkg/db/postgres/project.go | 7 ---- backend/pkg/messages/primitives.go | 2 +- backend/pkg/url/assets/url.go | 2 +- backend/services/http/handlers.go | 4 +- backend/services/http/handlers_ios.go | 4 +- 9 files changed, 65 insertions(+), 64 deletions(-) diff --git a/backend/pkg/db/cache/messages_common.go b/backend/pkg/db/cache/messages_common.go index 0b7d9a885..c05422cb2 100644 --- a/backend/pkg/db/cache/messages_common.go +++ b/backend/pkg/db/cache/messages_common.go @@ -28,3 +28,52 @@ func (c *PGCache) InsertIssueEvent(sessionID uint64, crash *IssueEvent) error { } return c.Conn.InsertIssueEvent(sessionID, session.ProjectID, crash) } + + +func (c *PGCache) InsertUserID(sessionID uint64, userID *IOSUserID) error { + if err := c.Conn.InsertIOSUserID(sessionID, userID); err != nil { + return err + } + session, err := c.GetSession(sessionID) + if err != nil { + return err + } + session.UserID = &userID.Value + return nil +} + +func (c *PGCache) InsertUserAnonymousID(sessionID uint64, userAnonymousID *IOSUserAnonymousID) error { + if err := c.Conn.InsertIOSUserAnonymousID(sessionID, userAnonymousID); err != nil { + return err + } + session, err := c.GetSession(sessionID) + if err != nil { + return err + } + session.UserAnonymousID = &userAnonymousID.Value + return nil +} + +func (c *PGCache) InsertMetadata(sessionID uint64, metadata *Metadata) error { + session, err := c.GetSession(sessionID) + if err != nil { + return err + } + project, err := c.GetProject(session.ProjectID) + if err != nil { + return err + } + + keyNo := project.GetMetadataNo(metadata.Key) + + if keyNo == 0 { + // insert project metadata + } + + if err := c.Conn.InsertMetadata(sessionID, keyNo, metadata.Value); err != nil { + return err + } + + session.SetMetadata(keyNo, metadata.Value) + return nil +} diff --git a/backend/pkg/db/cache/messages_ios.go b/backend/pkg/db/cache/messages_ios.go index 151ffe58e..f630de53d 100644 --- a/backend/pkg/db/cache/messages_ios.go +++ b/backend/pkg/db/cache/messages_ios.go @@ -95,46 +95,3 @@ func (c *PGCache) InsertIOSIssueEvent(sessionID uint64, issueEvent *IOSIssueEven return nil } -func (c *PGCache) InsertUserID(sessionID uint64, userID *IOSUserID) error { - if err := c.Conn.InsertIOSUserID(sessionID, userID); err != nil { - return err - } - session, err := c.GetSession(sessionID) - if err != nil { - return err - } - session.UserID = &userID.Value - return nil -} - -func (c *PGCache) InsertUserAnonymousID(sessionID uint64, userAnonymousID *IOSUserAnonymousID) error { - if err := c.Conn.InsertIOSUserAnonymousID(sessionID, userAnonymousID); err != nil { - return err - } - session, err := c.GetSession(sessionID) - if err != nil { - return err - } - session.UserAnonymousID = &userAnonymousID.Value - return nil -} - -func (c *PGCache) InsertMetadata(sessionID uint64, metadata *Metadata) error { - session, err := c.GetSession(sessionID) - if err != nil { - return err - } - project, err := c.GetProject(session.ProjectID) - if err != nil { - return err - } - - keyNo := project.GetMetadataNo(metadata.Key) - if err := c.Conn.InsertMetadata(sessionID, keyNo, metadata.Value); err != nil { - return err - } - - session.SetMetadata(keyNo, metadata.Value) - return nil -} - diff --git a/backend/pkg/db/cache/project.go b/backend/pkg/db/cache/project.go index dacb46633..1411e608b 100644 --- a/backend/pkg/db/cache/project.go +++ b/backend/pkg/db/cache/project.go @@ -11,7 +11,7 @@ func (c *PGCache) GetProjectByKey(projectKey string) (*Project, error) { return c.projectsByKeys[ projectKey ].Project, nil } p, err := c.Conn.GetProjectByKey(projectKey) - if p == nil { + if err != nil { return nil, err } c.projectsByKeys[ projectKey ] = &ProjectMeta{ p, time.Now().Add(c.projectExpirationTimeout) } @@ -27,7 +27,7 @@ func (c *PGCache) GetProject(projectID uint32) (*Project, error) { return c.projects[ projectID ].Project, nil } p, err := c.Conn.GetProject(projectID) - if p == nil { + if err != nil { return nil, err } c.projects[ projectID ] = &ProjectMeta{ p, time.Now().Add(c.projectExpirationTimeout) } diff --git a/backend/pkg/db/postgres/errors.go b/backend/pkg/db/postgres/errors.go index 9012bfe6b..a83c8f03a 100644 --- a/backend/pkg/db/postgres/errors.go +++ b/backend/pkg/db/postgres/errors.go @@ -2,15 +2,17 @@ package postgres import ( "errors" - + + "github.com/jackc/pgx/v4" "github.com/jackc/pgconn" "github.com/jackc/pgerrcode" ) func IsPkeyViolation(err error) bool { var pgErr *pgconn.PgError - if errors.As(err, &pgErr) && pgErr.Code == pgerrcode.UniqueViolation { - return true - } - return false -} \ No newline at end of file + return errors.As(err, &pgErr) && pgErr.Code == pgerrcode.UniqueViolation +} + +func IsNoRowsErr(err error) bool { + return err == pgx.ErrNoRows +} diff --git a/backend/pkg/db/postgres/project.go b/backend/pkg/db/postgres/project.go index 461db66fb..2eea30662 100644 --- a/backend/pkg/db/postgres/project.go +++ b/backend/pkg/db/postgres/project.go @@ -1,7 +1,6 @@ package postgres import ( - "github.com/jackc/pgx/v4" . "openreplay/backend/pkg/db/types" ) @@ -14,9 +13,6 @@ func (conn *Conn) GetProjectByKey(projectKey string) (*Project, error) { `, projectKey, ).Scan(&p.MaxSessionDuration, &p.SampleRate, &p.ProjectID); err != nil { - if err == pgx.ErrNoRows { - err = nil - } return nil, err } return p, nil @@ -36,9 +32,6 @@ func (conn *Conn) GetProject(projectID uint32) (*Project, error) { ).Scan(&p.ProjectKey,&p.MaxSessionDuration, &p.Metadata1, &p.Metadata2, &p.Metadata3, &p.Metadata4, &p.Metadata5, &p.Metadata6, &p.Metadata7, &p.Metadata8, &p.Metadata9, &p.Metadata10); err != nil { - if err == pgx.ErrNoRows { - err = nil - } return nil, err } return p, nil diff --git a/backend/pkg/messages/primitives.go b/backend/pkg/messages/primitives.go index 0c938d2b2..70952eeab 100644 --- a/backend/pkg/messages/primitives.go +++ b/backend/pkg/messages/primitives.go @@ -49,7 +49,7 @@ func ReadUint(reader io.Reader) (uint64, error) { } if b < 0x80 { if i > 9 || i == 9 && b > 1 { - return x, errors.New("overflow") + return x, errors.New("uint overflow") } return x | uint64(b)<>50, 10) } diff --git a/backend/services/http/handlers.go b/backend/services/http/handlers.go index 975abe31b..81cd6e9c7 100644 --- a/backend/services/http/handlers.go +++ b/backend/services/http/handlers.go @@ -57,8 +57,8 @@ func startSessionHandlerWeb(w http.ResponseWriter, r *http.Request) { } p, err := pgconn.GetProjectByKey(*req.ProjectKey) - if p == nil { - if err == nil { + if err != nil { + if postgres.IsNoRowsErr(err) { responseWithError(w, http.StatusNotFound, errors.New("Project doesn't exist or is not active")) } else { responseWithError(w, http.StatusInternalServerError, err) // TODO: send error here only on staging diff --git a/backend/services/http/handlers_ios.go b/backend/services/http/handlers_ios.go index 2c874a312..32f4a271a 100644 --- a/backend/services/http/handlers_ios.go +++ b/backend/services/http/handlers_ios.go @@ -51,8 +51,8 @@ package main // return // } // p, err := pgconn.GetProject(uint32(projectID)) -// if p == nil { -// if err == nil { +// if err != nil { +// if postgres.IsNoRowsErr(err) { // responseWithError(w, http.StatusNotFound, errors.New("Project doesn't exist or is not active")) // } else { // responseWithError(w, http.StatusInternalServerError, err) // TODO: send error here only on staging