fix: obtained token is empty #144
Conversation
WalkthroughThis pull request replaces the custom scripts/wait-for.sh utility with the npm package wait-on (added to dependencies with corresponding TypeScript types). package.json updated accordingly. Seed scripts no longer prefix commands with NODE_ENV=development. scripts/auth.sh now always sources the development env file. scripts/init.sh removes an interactive confirm and switches wait calls to npx wait-on. test/e2e global.setup.ts now uses wait-on with millisecond timeouts. The scripts/wait-for.sh file was removed. Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)scripts/auth.sh (1)
🪛 Shellcheck (0.11.0)scripts/auth.sh[warning] 8-8: Declare and assign separately to avoid masking return values. (SC2155) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/e2e/global.setup.ts (1)
20-25: Use explicit timeout error matching
wait-on v9.0.1 throws a plain Error with message
“Timed out waiting for: ” (noerr.code). Match the full prefix instead of a generic substring:} catch (err) { - if (err.message.includes('Timed out')) { + if (err.message?.startsWith('Timed out waiting for: ')) { console.log(`Service at ${url} not running. Starting…`); } else { throw err;
🧹 Nitpick comments (2)
scripts/auth.sh (1)
6-6: Declare and assign separately to avoid masking return values.Combining declaration and assignment in a single statement with command substitution masks the return value of the command. If
curlorjqfails, the function will still return success.This is addressed by the suggested fix in the previous comment, which separates the curl response capture from the token extraction.
scripts/init.sh (1)
10-14: Declare and assign separately to avoid masking return values.Lines 11-12 combine declaration and command substitution in single statements, which masks the return value. If
curlfails despite retries, or ifjqfails, the function will still succeed and potentially return an empty token.Consider separating declaration from assignment:
function get_token { - local response=$(curl --fail --retry 5 -X POST -H "Content-Type: application/json" -d '{"email": "'"$ADMIN_EMAIL"'", "password": "'"$ADMIN_PASSWORD"'"}' $DIRECTUS_URL/auth/login) - local token=$(echo "$response" | jq -r '.data.access_token') + local response + local token + response=$(curl --fail --retry 5 -X POST -H "Content-Type: application/json" -d '{"email": "'"$ADMIN_EMAIL"'", "password": "'"$ADMIN_PASSWORD"'"}' $DIRECTUS_URL/auth/login) + token=$(echo "$response" | jq -r '.data.access_token') echo "$token" }This allows the script to catch curl/jq failures properly via
set -e.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
package.json(3 hunks)scripts/auth.sh(1 hunks)scripts/init.sh(3 hunks)scripts/wait-for.sh(0 hunks)test/e2e/global.setup.ts(1 hunks)
💤 Files with no reviewable changes (1)
- scripts/wait-for.sh
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}
⚙️ CodeRabbit configuration file
We use Nuxt with auto imports enabled. Don't warn about missing imports.
Files:
test/e2e/global.setup.ts
🧬 Code graph analysis (1)
scripts/auth.sh (1)
scripts/init.sh (1)
get_token(10-14)
🪛 Shellcheck (0.11.0)
scripts/auth.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 6-6: Declare and assign separately to avoid masking return values.
(SC2155)
scripts/init.sh
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 12-12: Declare and assign separately to avoid masking return values.
(SC2155)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: Run e2e tests
The problem was
get_tokenreturning:{"errors":[{"message":"Service \"api\" is unavailable. Under pressure.","extensions":{"service":"api","reason":"Under pressure","code":"SERVICE_UNAVAILABLE"}}]}. So I've added retry to curl, + replaced wait-for lib with the one waiting for 2XX response.