From 5854a493ec5d809f4f4ab359fb9e9212d7c6ff92 Mon Sep 17 00:00:00 2001 From: Almond Heil Date: Fri, 14 Nov 2025 08:48:38 -0700 Subject: [PATCH 1/9] Separate param building and call it before outputting JSON Signed-off-by: Almond Heil --- cmd/boot-script-service/default_api.go | 67 ++++++++++++++++++-------- 1 file changed, 47 insertions(+), 20 deletions(-) diff --git a/cmd/boot-script-service/default_api.go b/cmd/boot-script-service/default_api.go index 7e6812d..36cf684 100644 --- a/cmd/boot-script-service/default_api.go +++ b/cmd/boot-script-service/default_api.go @@ -643,6 +643,39 @@ func buildBootScript(bd BootData, sp scriptParams, chain, role, subRole, descr s return "", fmt.Errorf("%s: this host not configured for booting.", descr) } + script := "#!ipxe\n" + params, err := buildParams(bd, sp, role, subRole) + if err != nil { + // TODO: HANDLE PARAM BUILDING ERROR HERE. + return "", fmt.Errorf("need to figure out how to handle this error") + } + + u := bd.Kernel.Path + u, err = checkURL(u) + if err == nil { + script += "kernel --name kernel " + u + " " + strings.Trim(params, " ") + script += " || goto boot_retry\n" + if bd.Initrd.Path != "" { + u, err = checkURL(bd.Initrd.Path) + if err == nil { + script += "initrd --name initrd " + u + " || goto boot_retry\n" + } + } + script += "boot || goto boot_retry\n:boot_retry\n" + // We could vary the length of the sleep based on retry count or some + // other criteria. + // For now, just sleep a bit + script += fmt.Sprintf("sleep %d\n", retryDelay) + chain + "\n" + } + return script, err +} + +// Function buildParams() constructs the full parameter list based on the +// BootData and additional parameters provided, accounting for special +// parameters. The params are returned as a string. If an error occurs, an +// empty string is returned along with the error. +func buildParams(bd BootData, sp scriptParams, role, subRole string) (string, error) { + debugf("buildParams(%v, %v, %v, %v)", bd, sp, role, subRole) params := bd.Params if bd.Kernel.Params != "" { params += " " + bd.Kernel.Params @@ -677,7 +710,7 @@ func buildBootScript(bd BootData, sp scriptParams, chain, role, subRole, descr s err = nil } - script := "#!ipxe\n" + // TODO: This is confusing, should add a comment over this. if bd.Initrd.Path != "" { start := strings.Index(params, "initrd") if start != -1 { @@ -689,24 +722,8 @@ func buildBootScript(bd BootData, sp scriptParams, chain, role, subRole, descr s } params = "initrd=initrd " + params } - u := bd.Kernel.Path - u, err = checkURL(u) - if err == nil { - script += "kernel --name kernel " + u + " " + strings.Trim(params, " ") - script += " || goto boot_retry\n" - if bd.Initrd.Path != "" { - u, err = checkURL(bd.Initrd.Path) - if err == nil { - script += "initrd --name initrd " + u + " || goto boot_retry\n" - } - } - script += "boot || goto boot_retry\n:boot_retry\n" - // We could vary the length of the sleep based on retry count or some - // other criteria. - // For now, just sleep a bit - script += fmt.Sprintf("sleep %d\n", retryDelay) + chain + "\n" - } - return script, err + + return params, nil } // Function unknownBootScript() constructs the boot script for an unknown host @@ -825,14 +842,25 @@ func BootscriptGet(w http.ResponseWriter, r *http.Request) { log.Printf("BSS request failed: bootscript request without mac=, name=, or nid= parameter") return } + sp := scriptParams{comp.ID, comp.NID.String(), bd.ReferralToken} debugf("bd: %v\n", bd) debugf("comp: %v\n", comp) is_json, _ := getIntParam(r, "json", 0) if is_json != 0 { + params, err := buildParams(bd, sp, comp.Role, comp.SubRole) + if err != nil { + // TODO: handling this error may be a lot of logic, I should figure that out. could intersect w/ updating state. + message := fmt.Sprintf("Failed to build params, but I can't fully describe yet: %v", err) + base.SendProblemDetailsGeneric(w, http.StatusInternalServerError, message) + return + } + + bd.Params = "kernel " + params b, err := json.Marshal(bd) if err != nil { + // TODO: if the SendProblemDetailsGeneric works above maybe should do it here too. there's only 2 places where this way of printing error is used. w.WriteHeader(http.StatusInternalServerError) fmt.Fprintf(w, "Failed to marshal JSON response: %v", err) return @@ -891,7 +919,6 @@ func BootscriptGet(w http.ResponseWriter, r *http.Request) { if mac == "" && comp.Mac != nil { mac = comp.Mac[0] } - sp := scriptParams{comp.ID, comp.NID.String(), bd.ReferralToken} chain := "chain " + chainProto + "://" + ipxeServer + gwURI + r.URL.Path if mac != "" { chain += "?mac=" + mac From 6e2875ccc3acc243bbc6447b953023282dd3cd26 Mon Sep 17 00:00:00 2001 From: Almond Heil Date: Mon, 17 Nov 2025 11:53:26 -0700 Subject: [PATCH 2/9] Resolve TODOs from my development Signed-off-by: Almond Heil --- cmd/boot-script-service/default_api.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/cmd/boot-script-service/default_api.go b/cmd/boot-script-service/default_api.go index 36cf684..feb2a16 100644 --- a/cmd/boot-script-service/default_api.go +++ b/cmd/boot-script-service/default_api.go @@ -646,8 +646,7 @@ func buildBootScript(bd BootData, sp scriptParams, chain, role, subRole, descr s script := "#!ipxe\n" params, err := buildParams(bd, sp, role, subRole) if err != nil { - // TODO: HANDLE PARAM BUILDING ERROR HERE. - return "", fmt.Errorf("need to figure out how to handle this error") + return "", err } u := bd.Kernel.Path @@ -710,7 +709,7 @@ func buildParams(bd BootData, sp scriptParams, role, subRole string) (string, er err = nil } - // TODO: This is confusing, should add a comment over this. + // if bootdata specifies an initrd, add "initrd=initrd" to params (deleting any existing occurrence) if bd.Initrd.Path != "" { start := strings.Index(params, "initrd") if start != -1 { @@ -851,8 +850,7 @@ func BootscriptGet(w http.ResponseWriter, r *http.Request) { if is_json != 0 { params, err := buildParams(bd, sp, comp.Role, comp.SubRole) if err != nil { - // TODO: handling this error may be a lot of logic, I should figure that out. could intersect w/ updating state. - message := fmt.Sprintf("Failed to build params, but I can't fully describe yet: %v", err) + message := fmt.Sprintf("Failed to build params: %v", err) base.SendProblemDetailsGeneric(w, http.StatusInternalServerError, message) return } @@ -860,7 +858,6 @@ func BootscriptGet(w http.ResponseWriter, r *http.Request) { bd.Params = "kernel " + params b, err := json.Marshal(bd) if err != nil { - // TODO: if the SendProblemDetailsGeneric works above maybe should do it here too. there's only 2 places where this way of printing error is used. w.WriteHeader(http.StatusInternalServerError) fmt.Fprintf(w, "Failed to marshal JSON response: %v", err) return From 4d86b1b62f4eed1b34cea7e404fbd9bac23102c1 Mon Sep 17 00:00:00 2001 From: Almond Heil Date: Tue, 18 Nov 2025 08:50:55 -0700 Subject: [PATCH 3/9] Add bootscript JSON output example to docs This example return value is based on the example above, but neither is completely accurate to what BSS currently does with its params. In particular, these examples lack the params initrd, mac, nid, and ds which get added automatically in buildBootScript(). Signed-off-by: Almond Heil --- docs/examples.adoc | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/docs/examples.adoc b/docs/examples.adoc index 0e81eb9..384545c 100644 --- a/docs/examples.adoc +++ b/docs/examples.adoc @@ -103,6 +103,28 @@ Identify the node which expects to receive an iPXE boot script. ---- +[source, bash] +.Use curl to request boot script components in JSON +---- + + curl 'https://sms-1/apis/bss/boot/v1/bootscript?mac=44:A8:42:21:A8:AD&json=1' +---- + +[source, json] +.A JSON object similar to the following will be returned: +---- + + { + "params": "kernel bootname=x0c0s7b0n0 console=ttyS0,115200 console=tty0 unregistered=1 heartbeat_url=http://sms-1/apis/hbtd/heartbeat bootmac=44:A8:42:21:A8:AD", + "kernel": { + "path": "http://sms-1/apis/ars/assets/artifacts/generic/vmlinuz_801" + }, + "initrd": { + "path": "http://sms-1/apis/ars/assets/artifacts/generic/initramfs-cray_1058.img" + } + } +---- + === Retrieve current boot parameters for one or more nodes Make a GET request to /bootparameters endpoint. Specifiy a list of nodes to retrieve parameters for. If no nodes are given, will return all nodes which currently have parameter settings. From 256231ba34f9207df3d7c81df7c0bb9d5b1ab0f3 Mon Sep 17 00:00:00 2001 From: Almond Heil Date: Tue, 18 Nov 2025 11:31:46 -0700 Subject: [PATCH 4/9] Add json query param to swagger doc with TODO about schema Signed-off-by: Almond Heil --- api/swagger.yaml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/api/swagger.yaml b/api/swagger.yaml index ff518dd..9796857 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -209,6 +209,15 @@ paths: in: query type: integer description: Node ID (NID) of host requesting boot script + - name: json + in: query + type: integer + default: 0 + description: >- + If nonzero, a JSON object will be returned instead of an iPXE boot script. + TODO: it would make sense to give a schema for this object, + but Swagger doesn't expect a query parameter to change the output + schema because this is a weird way to implement it. - name: retry in: query type: integer From 91e024595fb60bb59f46f65909da7072eae2de94 Mon Sep 17 00:00:00 2001 From: Almond Heil Date: Tue, 18 Nov 2025 15:51:29 -0700 Subject: [PATCH 5/9] Unit tests for buildParams success Signed-off-by: Almond Heil --- cmd/boot-script-service/default_api_test.go | 103 ++++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/cmd/boot-script-service/default_api_test.go b/cmd/boot-script-service/default_api_test.go index bbdd97d..9a8b8c7 100644 --- a/cmd/boot-script-service/default_api_test.go +++ b/cmd/boot-script-service/default_api_test.go @@ -186,6 +186,109 @@ func TestReplaceS3Params_error(t *testing.T) { } } +func TestBuildParams_Success(t *testing.T) { + test_cases := []struct { + bd BootData + sp scriptParams + expected string + }{ + { + // basic usage + BootData{ + Params: "root=nfs:example/path/to/rootfs:ro", + Kernel: ImageData{Path: "http://example/path/to/vmlinuz"}, + Initrd: ImageData{Path: "http://example/path/to/initramfs.img"}, + }, + scriptParams{ + xname: "x0000c0s0b0n0", + nid: "0", + }, + fmt.Sprintf("initrd=initrd root=nfs:example/path/to/rootfs:ro xname=x0000c0s0b0n0 nid=0 ds=nocloud-net;s=%s/", advertiseAddress), + }, + { + // bss referral token + BootData{ + Params: "root=nfs:example/path/to/rootfs:ro", + Kernel: ImageData{Path: "http://example/path/to/vmlinuz"}, + Initrd: ImageData{Path: "http://example/path/to/initramfs.img"}, + }, + scriptParams{ + xname: "x0000c0s0b0n0", + nid: "0", + referralToken: "meaningless_but_nonempty_string", + }, + fmt.Sprintf("initrd=initrd root=nfs:example/path/to/rootfs:ro xname=x0000c0s0b0n0 nid=0 bss_referral_token=meaningless_but_nonempty_string ds=nocloud-net;s=%s/", advertiseAddress), + }, + { + // params set under kernel + BootData{ + Params: "root=nfs:example/path/to/rootfs:ro", + Kernel: ImageData{Path: "http://example/path/to/vmlinuz", Params: "kernel_param=set"}, + Initrd: ImageData{Path: "http://example/path/to/initramfs.img"}, + }, + scriptParams{ + xname: "x0000c0s0b0n0", + nid: "0", + }, + fmt.Sprintf("initrd=initrd root=nfs:example/path/to/rootfs:ro kernel_param=set xname=x0000c0s0b0n0 nid=0 ds=nocloud-net;s=%s/", advertiseAddress), + }, + { + // params set under initrd + BootData{ + Params: "root=nfs:example/path/to/rootfs:ro", + Kernel: ImageData{Path: "http://example/path/to/vmlinuz"}, + Initrd: ImageData{Path: "http://example/path/to/initramfs.img", Params: "init_param=set"}, // not initrd_param because a find replace kills it. TODO: should that behavior be classified a bug? + }, + scriptParams{ + xname: "x0000c0s0b0n0", + nid: "0", + }, + fmt.Sprintf("initrd=initrd root=nfs:example/path/to/rootfs:ro init_param=set xname=x0000c0s0b0n0 nid=0 ds=nocloud-net;s=%s/", advertiseAddress), + }, + { + // initrd already in bootdata params + BootData{ + Params: "initrd=should_get_deleted root=nfs:example/path/to/rootfs:ro", + Kernel: ImageData{Path: "http://example/path/to/vmlinuz"}, + Initrd: ImageData{Path: "http://example/path/to/initramfs.img"}, + }, + scriptParams{ + xname: "x0000c0s0b0n0", + nid: "0", + }, + fmt.Sprintf("initrd=initrd root=nfs:example/path/to/rootfs:ro xname=x0000c0s0b0n0 nid=0 ds=nocloud-net;s=%s/", advertiseAddress), + }, + { + // initrd already in bootdata params (at the end) + BootData{ + Params: "root=nfs:example/path/to/rootfs:ro initrd=should_get_deleted", + Kernel: ImageData{Path: "http://example/path/to/vmlinuz"}, + Initrd: ImageData{Path: "http://example/path/to/initramfs.img"}, + }, + scriptParams{ + xname: "x0000c0s0b0n0", + nid: "0", + }, + fmt.Sprintf("initrd=initrd root=nfs:example/path/to/rootfs:ro xname=x0000c0s0b0n0 nid=0 ds=nocloud-net;s=%s/", advertiseAddress), + }, + } + + for _, tc := range test_cases { + // role and subRole are only used when adding spire jopin tokens, which we can't test for this way. + output, err := buildParams(tc.bd, tc.sp, "dummy role", "dummy subRole") + if err != nil { + t.Errorf("Failed to build params: %v\n", err) + } + + if output != tc.expected { + t.Log("Built params did not match.") + t.Logf(" expected: %s", tc.expected) + t.Logf(" got: %s", output) + t.Fail() + } + } +} + func TestBootparametersPost_Success(t *testing.T) { args := bssTypes.BootParams{ From 4b77c1a56d043a055beea5852172ffb4b766cb25 Mon Sep 17 00:00:00 2001 From: Almond Heil Date: Wed, 19 Nov 2025 10:31:23 -0700 Subject: [PATCH 6/9] Fix initrd replacement tests by adding expected whitespace Signed-off-by: Almond Heil --- cmd/boot-script-service/default_api_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/boot-script-service/default_api_test.go b/cmd/boot-script-service/default_api_test.go index 9a8b8c7..07fc0ea 100644 --- a/cmd/boot-script-service/default_api_test.go +++ b/cmd/boot-script-service/default_api_test.go @@ -256,7 +256,7 @@ func TestBuildParams_Success(t *testing.T) { xname: "x0000c0s0b0n0", nid: "0", }, - fmt.Sprintf("initrd=initrd root=nfs:example/path/to/rootfs:ro xname=x0000c0s0b0n0 nid=0 ds=nocloud-net;s=%s/", advertiseAddress), + fmt.Sprintf("initrd=initrd root=nfs:example/path/to/rootfs:ro xname=x0000c0s0b0n0 nid=0 ds=nocloud-net;s=%s/", advertiseAddress), }, { // initrd already in bootdata params (at the end) @@ -269,7 +269,7 @@ func TestBuildParams_Success(t *testing.T) { xname: "x0000c0s0b0n0", nid: "0", }, - fmt.Sprintf("initrd=initrd root=nfs:example/path/to/rootfs:ro xname=x0000c0s0b0n0 nid=0 ds=nocloud-net;s=%s/", advertiseAddress), + fmt.Sprintf("initrd=initrd root=nfs:example/path/to/rootfs:ro xname=x0000c0s0b0n0 nid=0 ds=nocloud-net;s=%s/", advertiseAddress), }, } From 80831bb2d0fe57c43327f67470c4050b7dbb029d Mon Sep 17 00:00:00 2001 From: Almond Heil Date: Wed, 19 Nov 2025 13:37:49 -0700 Subject: [PATCH 7/9] Treat expected params as a list of strings Signed-off-by: Almond Heil --- cmd/boot-script-service/default_api_test.go | 69 ++++++++++++++++----- 1 file changed, 55 insertions(+), 14 deletions(-) diff --git a/cmd/boot-script-service/default_api_test.go b/cmd/boot-script-service/default_api_test.go index 07fc0ea..ba320fe 100644 --- a/cmd/boot-script-service/default_api_test.go +++ b/cmd/boot-script-service/default_api_test.go @@ -28,7 +28,9 @@ import ( "fmt" "net/http" "net/http/httptest" + "reflect" "regexp" + "strings" "testing" "github.com/OpenCHAMI/bss/pkg/bssTypes" @@ -188,9 +190,9 @@ func TestReplaceS3Params_error(t *testing.T) { func TestBuildParams_Success(t *testing.T) { test_cases := []struct { - bd BootData - sp scriptParams - expected string + bd BootData + sp scriptParams + expectedParams []string }{ { // basic usage @@ -203,7 +205,13 @@ func TestBuildParams_Success(t *testing.T) { xname: "x0000c0s0b0n0", nid: "0", }, - fmt.Sprintf("initrd=initrd root=nfs:example/path/to/rootfs:ro xname=x0000c0s0b0n0 nid=0 ds=nocloud-net;s=%s/", advertiseAddress), + []string{ + "initrd=initrd", + "root=nfs:example/path/to/rootfs:ro", + "xname=x0000c0s0b0n0", + "nid=0", + fmt.Sprintf("ds=nocloud-net;s=%s/", advertiseAddress), + }, }, { // bss referral token @@ -217,7 +225,14 @@ func TestBuildParams_Success(t *testing.T) { nid: "0", referralToken: "meaningless_but_nonempty_string", }, - fmt.Sprintf("initrd=initrd root=nfs:example/path/to/rootfs:ro xname=x0000c0s0b0n0 nid=0 bss_referral_token=meaningless_but_nonempty_string ds=nocloud-net;s=%s/", advertiseAddress), + []string{ + "initrd=initrd", + "root=nfs:example/path/to/rootfs:ro", + "xname=x0000c0s0b0n0", + "nid=0", + "bss_referral_token=meaningless_but_nonempty_string", + fmt.Sprintf("ds=nocloud-net;s=%s/", advertiseAddress), + }, }, { // params set under kernel @@ -230,20 +245,34 @@ func TestBuildParams_Success(t *testing.T) { xname: "x0000c0s0b0n0", nid: "0", }, - fmt.Sprintf("initrd=initrd root=nfs:example/path/to/rootfs:ro kernel_param=set xname=x0000c0s0b0n0 nid=0 ds=nocloud-net;s=%s/", advertiseAddress), + []string{ + "initrd=initrd", + "root=nfs:example/path/to/rootfs:ro", + "kernel_param=set", + "xname=x0000c0s0b0n0", + "nid=0", + fmt.Sprintf("ds=nocloud-net;s=%s/", advertiseAddress), + }, }, { // params set under initrd BootData{ Params: "root=nfs:example/path/to/rootfs:ro", Kernel: ImageData{Path: "http://example/path/to/vmlinuz"}, - Initrd: ImageData{Path: "http://example/path/to/initramfs.img", Params: "init_param=set"}, // not initrd_param because a find replace kills it. TODO: should that behavior be classified a bug? + Initrd: ImageData{Path: "http://example/path/to/initramfs.img", Params: "init_param=set"}, }, scriptParams{ xname: "x0000c0s0b0n0", nid: "0", }, - fmt.Sprintf("initrd=initrd root=nfs:example/path/to/rootfs:ro init_param=set xname=x0000c0s0b0n0 nid=0 ds=nocloud-net;s=%s/", advertiseAddress), + []string{ + "initrd=initrd", + "root=nfs:example/path/to/rootfs:ro", + "init_param=set", + "xname=x0000c0s0b0n0", + "nid=0", + fmt.Sprintf("ds=nocloud-net;s=%s/", advertiseAddress), + }, }, { // initrd already in bootdata params @@ -256,7 +285,13 @@ func TestBuildParams_Success(t *testing.T) { xname: "x0000c0s0b0n0", nid: "0", }, - fmt.Sprintf("initrd=initrd root=nfs:example/path/to/rootfs:ro xname=x0000c0s0b0n0 nid=0 ds=nocloud-net;s=%s/", advertiseAddress), + []string{ + "initrd=initrd", + "root=nfs:example/path/to/rootfs:ro", + "xname=x0000c0s0b0n0", + "nid=0", + fmt.Sprintf("ds=nocloud-net;s=%s/", advertiseAddress), + }, }, { // initrd already in bootdata params (at the end) @@ -269,8 +304,13 @@ func TestBuildParams_Success(t *testing.T) { xname: "x0000c0s0b0n0", nid: "0", }, - fmt.Sprintf("initrd=initrd root=nfs:example/path/to/rootfs:ro xname=x0000c0s0b0n0 nid=0 ds=nocloud-net;s=%s/", advertiseAddress), - }, + []string{ + "initrd=initrd", + "root=nfs:example/path/to/rootfs:ro", + "xname=x0000c0s0b0n0", + "nid=0", + fmt.Sprintf("ds=nocloud-net;s=%s/", advertiseAddress), + }}, } for _, tc := range test_cases { @@ -280,10 +320,11 @@ func TestBuildParams_Success(t *testing.T) { t.Errorf("Failed to build params: %v\n", err) } - if output != tc.expected { + outputParams := strings.Fields(output) + if !reflect.DeepEqual(tc.expectedParams, outputParams) { t.Log("Built params did not match.") - t.Logf(" expected: %s", tc.expected) - t.Logf(" got: %s", output) + t.Logf(" expected: %v", tc.expectedParams) + t.Logf(" got: %v", outputParams) t.Fail() } } From 1185903ce9420d5b734d84d5ba8cc039a705d052 Mon Sep 17 00:00:00 2001 From: Almond Heil Date: Wed, 19 Nov 2025 16:12:03 -0700 Subject: [PATCH 8/9] Make API TODO into a YAML comment Signed-off-by: Almond Heil --- api/swagger.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/swagger.yaml b/api/swagger.yaml index 9796857..66cb43e 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -213,11 +213,11 @@ paths: in: query type: integer default: 0 + # TODO: it would make sense to give a schema for this object, but + # Swagger doesn't expect a query parameter to change the output + # schema because this is a weird way to implement it. description: >- If nonzero, a JSON object will be returned instead of an iPXE boot script. - TODO: it would make sense to give a schema for this object, - but Swagger doesn't expect a query parameter to change the output - schema because this is a weird way to implement it. - name: retry in: query type: integer From 92c529c10f0eed7fdf9da30595f704085939a4da Mon Sep 17 00:00:00 2001 From: Almond Heil Date: Mon, 24 Nov 2025 11:10:56 -0700 Subject: [PATCH 9/9] Correct buildParams doc comment Signed-off-by: Almond Heil --- cmd/boot-script-service/default_api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/boot-script-service/default_api.go b/cmd/boot-script-service/default_api.go index feb2a16..dc779aa 100644 --- a/cmd/boot-script-service/default_api.go +++ b/cmd/boot-script-service/default_api.go @@ -669,7 +669,7 @@ func buildBootScript(bd BootData, sp scriptParams, chain, role, subRole, descr s return script, err } -// Function buildParams() constructs the full parameter list based on the +// buildParams constructs the full parameter list based on the // BootData and additional parameters provided, accounting for special // parameters. The params are returned as a string. If an error occurs, an // empty string is returned along with the error.