fix: Handle location header array with a single value without emitting warning

[donnabelsey]

Co-authored with @brunns
Related to #977

Recently a change was added that handled when the Location header is an array. However, when the array is of length 1 a warning is mistakenly printed in the logs.

[changeset-bot]

🦋 Changeset detected

Latest commit: c8959c0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@opennextjs/aws	Patch
app-pages-router	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vicb]

The changes LGTM but I'd like to understand how many values (bounds) this array can have.

Can we have 3, 4, ... values, what would they correspond to, could they be different?

@sommeeeer any idea about that?

[sommeeeer]

Can we have 3, 4, ... values, what would they correspond to, could they be different?

@sommeeeer any idea about that?

Thats why I asked about the same thing in this comment. I wasn’t able to reproduce this myself, so I’m wondering what the indices in the array correspond to. If they don’t match the elements we expect, we’ll need to dig further into this issue.

[vicb]

Can we have 3, 4, ... values, what would they correspond to, could they be different?
@sommeeeer any idea about that?

Thats why I asked about the same thing in this comment. I wasn’t able to reproduce this myself, so I’m wondering what the indices in the array correspond to. If they don’t match the elements we expect, we’ll need to dig further into this issue.

At least merging this PR should not harm but we should keep investigating to understand how this happen (i.e. create a new issue for that). WDYT?

[sommeeeer]

At least merging this PR should not harm but we should keep investigating to understand how this happen (i.e. create a new issue for that). WDYT?

Agree, I'll create one right away.

Update: Are we sure we want to do a || in the conditional THO? What if the elements are not equal? We probably want to see that.

[vicb]

Update: Are we sure we want to do a || in the conditional THO? What if the elements are not equal? We probably want to see that.

If think value.length === 1 || value[0] === value[1] is right. We test element equality when the array length is != 1

[sommeeeer]

Update: Are we sure we want to do a || in the conditional THO? What if the elements are not equal? We probably want to see that.

If think value.length === 1 || value[0] === value[1] is right. We test element equality when the array length is != 1

Yeah, LGTM. Forget what I said.

[pkg-pr-new]

Open in StackBlitz

pnpm add https://pkg.pr.new/@opennextjs/aws@1016

commit: c8959c0

[sommeeeer]

Thanks a lot for the PR @donnabelsey !

[vicb]

Thanks for the PR

[donnabelsey]

Can we have 3, 4, ... values, what would they correspond to, could they be different?
@sommeeeer any idea about that?

Thats why I asked about the same thing in this comment. I wasn’t able to reproduce this myself, so I’m wondering what the indices in the array correspond to. If they don’t match the elements we expect, we’ll need to dig further into this issue.

Thanks for looking at this, and I should've provided a bit more info about the investigation, apologies.

As suggested on the other thread by @sommeeeer we added the location array length and contents to the warning log; we consistently saw an array containing a single element which was a URL related to parts of the authentication flow. We are using the next-auth v5 library for managing authentication, and the redirects occur in the library from callbacks when certain parts of the login / logout flow happen.

We didn't investigate any further than that; in our case I wouldn't expect the array to contain any more than one element and the solution of selecting the last element as the location functionally still worked, the user still ended up in the right place.