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

Create callback returns 403 when promise already completed #502

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 21 additions & 15 deletions internal/app/coroutines/createCallback.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ func CreateCallback(c gocoro.Coroutine[*t_aio.Submission, *t_aio.Completion, any
return nil, t_api.NewError(t_api.StatusAIOStoreError, err)
}

// If the callback was already created return 200 and an empty callback
var cb *callback.Callback
status := t_api.StatusOK

if p.State == promise.Pending {
// If the callback was already created return 200 and an empty callback
var cb *callback.Callback
status := t_api.StatusOK

mesg := &message.Mesg{
Type: message.Resume,
Root: r.CreateCallback.RootPromiseId,
Expand Down Expand Up @@ -112,19 +112,25 @@ func CreateCallback(c gocoro.Coroutine[*t_aio.Submission, *t_aio.Completion, any
CreatedOn: createdOn,
}
}
}

res = &t_api.Response{
Kind: t_api.CreateCallback,
Tags: r.Tags,
CreateCallback: &t_api.CreateCallbackResponse{
// Status could be StatusOk or StatusCreated if the Callback Id was already present
Status: status,
Callback: cb,
Promise: p,
},
res = &t_api.Response{
Kind: t_api.CreateCallback,
Tags: r.Tags,
CreateCallback: &t_api.CreateCallbackResponse{
Status: status, // StatusCreated if the callback was created, otherwise StatusOk
Callback: cb,
Promise: p,
},
}
} else {
res = &t_api.Response{
Kind: t_api.CreateCallback,
Tags: r.Tags,
CreateCallback: &t_api.CreateCallbackResponse{
Status: alreadyCompletedStatus(p.State),
},
}
}

} else {
res = &t_api.Response{
Kind: t_api.CreateCallback,
Expand Down
2 changes: 1 addition & 1 deletion test/dst/dst.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@
func (d *DST) logNonLinearizable(ops []porcupine.Operation, history porcupine.LinearizationInfo) {
// log the linearizations
linearizations := history.PartialLinearizationsOperations()
util.Assert(len(linearizations) == 3, "linearizations must be equal to the number of partitions")
util.Assert(len(linearizations) == 4, "linearizations must be equal to the number of partitions")

Check warning on line 316 in test/dst/dst.go

View check run for this annotation

Codecov / codecov/patch

test/dst/dst.go#L316

Added line #L316 was not covered by tests
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just fixes a whoopsie that made it to main


// check each parition individually
// partitions are in order: promise, schedule, lock
Expand Down
32 changes: 32 additions & 0 deletions test/dst/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,38 @@

return model, nil

case t_api.StatusPromiseAlreadyResolved:
if p == nil {
return model, fmt.Errorf("promise '%s' does not exist", req.CreateCallback.PromiseId)
}

Check warning on line 337 in test/dst/validator.go

View check run for this annotation

Codecov / codecov/patch

test/dst/validator.go#L336-L337

Added lines #L336 - L337 were not covered by tests
if p.State != promise.Resolved && !(p.State == promise.Pending && p.Tags["resonate:timeout"] == "true" && resTime >= p.Timeout) {
return model, fmt.Errorf("promise '%s' not resolved", req.CreateCallback.PromiseId)
}

Check warning on line 340 in test/dst/validator.go

View check run for this annotation

Codecov / codecov/patch

test/dst/validator.go#L339-L340

Added lines #L339 - L340 were not covered by tests
return model, nil
case t_api.StatusPromiseAlreadyRejected:
if p == nil {
return model, fmt.Errorf("promise '%s' does not exist", req.CreateCallback.PromiseId)
}

Check warning on line 345 in test/dst/validator.go

View check run for this annotation

Codecov / codecov/patch

test/dst/validator.go#L344-L345

Added lines #L344 - L345 were not covered by tests
if p.State != promise.Rejected {
return model, fmt.Errorf("promise '%s' not rejected", req.CreateCallback.PromiseId)
}

Check warning on line 348 in test/dst/validator.go

View check run for this annotation

Codecov / codecov/patch

test/dst/validator.go#L347-L348

Added lines #L347 - L348 were not covered by tests
return model, nil
case t_api.StatusPromiseAlreadyCanceled:
if p == nil {
return model, fmt.Errorf("promise '%s' does not exist", req.CreateCallback.PromiseId)
}

Check warning on line 353 in test/dst/validator.go

View check run for this annotation

Codecov / codecov/patch

test/dst/validator.go#L352-L353

Added lines #L352 - L353 were not covered by tests
if p.State != promise.Canceled {
return model, fmt.Errorf("promise '%s' not canceled", req.CreateCallback.PromiseId)
}

Check warning on line 356 in test/dst/validator.go

View check run for this annotation

Codecov / codecov/patch

test/dst/validator.go#L355-L356

Added lines #L355 - L356 were not covered by tests
return model, nil
case t_api.StatusPromiseAlreadyTimedout:
if p == nil {
return model, fmt.Errorf("promise '%s' does not exist", req.CreateCallback.PromiseId)
}

Check warning on line 361 in test/dst/validator.go

View check run for this annotation

Codecov / codecov/patch

test/dst/validator.go#L360-L361

Added lines #L360 - L361 were not covered by tests
if p.State != promise.Timedout && !(p.State == promise.Pending && p.Tags["resonate:timeout"] != "true" && resTime >= p.Timeout) {
return model, fmt.Errorf("promise '%s' not timedout", req.CreateCallback.PromiseId)
}

Check warning on line 364 in test/dst/validator.go

View check run for this annotation

Codecov / codecov/patch

test/dst/validator.go#L363-L364

Added lines #L363 - L364 were not covered by tests
return model, nil
case t_api.StatusPromiseNotFound:
if p != nil {
return model, fmt.Errorf("promise '%s' exists", req.CreateCallback.PromiseId)
Expand Down