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

Link errors need to be wrapped in an amqp.DetachError #127

Closed
richardpark-msft opened this issue Feb 11, 2022 · 4 comments
Closed

Link errors need to be wrapped in an amqp.DetachError #127

richardpark-msft opened this issue Feb 11, 2022 · 4 comments
Labels
blocking-release Blocks release

Comments

@richardpark-msft
Copy link
Member

Currently network errors will leave the link in a detached state (which is fine) but will not indicate that detach state via the returned error. In paticular, this code in Receive() can be problematic:

func (r *Receiver) Receive(ctx context.Context) (*Message, error) {
	msg, err := r.Prefetched(ctx)

	if err != nil || msg != nil {
		return msg, err
	}

	// wait for the next message
	select {
	case msg := <-r.link.Messages:
		debug(3, "Receive() blocking %d", msg.deliveryID)
		msg.link = r.link
		return acceptIfModeFirst(ctx, r, &msg)
	case <-r.link.Detached:
                // NOTE: 
                // NOTE: r.link.err is not guaranteed to indicate the link is detached (ie, it can just be a network error)
                // NOTE: 
		return nil, r.link.err
	case <-ctx.Done():
		return nil, ctx.Err()
	}
}

The bit NOTE'd above will cause errors if you're using the error to determine if the link needs to be recovered. In general a caller should be able to see amqp.DetachError{}, for instance, and know they can do something useful here, like recreate just this link.

richardpark-msft pushed a commit to richardpark-msft/go-amqp that referenced this issue Feb 11, 2022
…link is detached.

This should make it easier for callers to figure out how to remediate the error.

Fixes Azure#127
@myusuf3
Copy link

myusuf3 commented Mar 15, 2022

is there a method to reestablish the link available to users of this library? on the sender side on stale connections.

@myusuf3
Copy link

myusuf3 commented Jun 1, 2022

just following up here. I am trying to update an upstream and hoping to avoid dealing with this at my level. How do you currently deal with stale connections on long lived receiver functions in worker environments?

@jhendrixMSFT jhendrixMSFT added the blocking-release Blocks release label Jun 24, 2022
@myusuf3
Copy link

myusuf3 commented Jul 10, 2022

@jhendrixMSFT I would contribute some effort into improving this does the team have a shared slack or something?

@jhendrixMSFT
Copy link
Member

Fixed in v0.18.0

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

Successfully merging a pull request may close this issue.

3 participants